[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [bug-gtypist] ideas, implementations, patches.

From: Felix Natter
Subject: Re: [bug-gtypist] ideas, implementations, patches.
Date: Fri, 08 Jul 2016 17:52:14 +0200
User-agent: Gnus/5.130006 (Ma Gnus v0.6) Emacs/24.4 (gnu/linux)

clutton <address@hidden> writes:

> On Tue, 2016-06-28 at 14:41 +0200, Felix Natter wrote:
>> clutton <address@hidden> writes:
> [snip]

hello clutton,

> TL/DR: Using "-D_XOPEN_SOURCE_EXTENDED -I/usr/include/ncursesw" and
> #include <curses.h>
> would resolve all ncursesw.h functionality.
> The meter of discussion is how to achieve that.


> ---------------answering to Felix----------------------
> Why my last patch works: It's the same case when you don't include
> <stdio.h> and use printf. The linker would find your printf eventually
> by linking to standard libraries. We at the end linking to libncursesw,
> although the processor know nothing, it works because linking resolve
> Unknown symbols. The approach is unacceptable, anyway.
> ----------------end---------------------------

At first I couldn't believe it, so I tried myself [1] and you're right

[1] for the record:
#include <curses.h>

int main()
    int ch;
    int retcode = get_wch(&ch);
    printf("ch = %d\n", ch);
    return 0;

$ gcc -o test test.c -lncursesw

$ gcc -o test test.c -lncurses
/tmp/cchTWg1l.o: In function `main':
test.c:(.text+0x1f): undefined reference to `get_wch'
collect2: error: ld returned 1 exit status

It's unacceptable, agreed.

> Take a look at my recent commit, I made it work with -Wall. -Wall
> -Werror are flags which should be used by developers while coding to
> avoid such situations.

For the record:
$ gcc -Wall -o test test.c -lncurses
test.c: In function ‘main’:
test.c:9:5: warning: implicit declaration of function ‘get_wch’ 
     int retcode = get_wch(&ch);

We should definitely use -Wall! Maybe hardening flags
(https://wiki.debian.org/Hardening) as well...

> For now, omit the configure.ac file, and push/[push after reviewing]
> the C code. Or should I make a cleaning up as a final step?

Ok, here's a review:

- configure.ac: looks good, but do we need to add it to CFLAGS as well?
  (why is CPPFLAGS honored by plain gcc anyway?)

- of course cmdline.[ch] and getopt.c are not under our control, but I
  realize the changes are necessary to be able to work with -Wall.

- gtypist.c, infoview.c, script.h, utf8.c:
  If a variable isn't used, why not just remove (instead of uncomment)
  the declaration?
Also, do we want to enable "-Wall" in configure.ac (maybe -ansi
-pedantic as well)? This might break compilation with non-gcc compilers
though, so it's probably better to do this for development only.

Once this is fixed (should be easy), do you want to merge the branch
yourself (no conflicts)?
$ git checkout master
$ git merge autoall
$ <add+commit src/ChangeLog entry>

Next time it would be good to have cleaner branches (i.e. do not name
commits "curses.h testing").

[2] Tim added i18n support for GNU gengetopt, which unfortunately is not
released yet, so I commited the files to git temporarily.

BTW: Shall I commit this change? I think you've said that it
fixes your problem?

--- a/src/Makefile.am
+++ b/src/Makefile.am
-gtypist_LDADD = @LIBINTL@ # TODO: do we need @LIBICONV@ here?

> Back to topic, after hacking a little I've found that nowadays projects
> resolve such issues by using pkg-config. With pkg-config it'll awfully
> simple to do proper checks. Something like:
> pkg-config --cflags-only-I ncursesw
> pkg-config --cflags ncursesw
> People hack it like that
> ncurses=ncursesw],
>   [AC_CHECK_LIB([ncursesw],
>                [initscr],
>                [LIBS="$LIBS -lncursesw"; ncurses=ncursesw],
>                [ncurses=ncurses])
> ])
> But, adding another dependence... What do you think? Should we left
> everything as it is now? Or use some hack instead of pkg-config?
> ncursesw has it's own utility for such a thing:ncursesw6-
> config, ncursesw5-config. But I vote against using them.

We've discussed using pkg-config before (after David from gentoo
reported that cbreak() sometimes is in -ltinfo):

See the thread "[bug-gtypist] gtypist fails to build with ncurses[tinfo]":
(especially the resolution:

BTW: We also need to make sure that gentoo compilation still works,
but I think it should.

Cheers and Best Regards,
Felix Natter

reply via email to

[Prev in Thread] Current Thread [Next in Thread]