Discussion:
Bug#868609: le FTBFS with latest ncurses
Add Reply
Alexander V. Lukyanov
2017-07-17 09:00:03 UTC
Reply
Permalink
Raw Message
Source: le
Version: 1.16.3-1
Severity: serious
Tags: buster sid
https://tests.reproducible-builds.org/debian/rb-pkg/unstable/amd64/le.html
...
gcc -DHAVE_CONFIG_H -I. -I../lib -I../lib -I../lib -I/usr/include/ncursesw -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wwrite-strings -Woverloaded-virtual -fno-exceptions -fno-rtti -fno-implement-inlines -c -o color.o color.cc
color.cc:36:12: error: 'int find_pair(int, int)' was declared 'extern' and later 'static' [-fpermissive]
static int find_pair(int fg,int bg)
^~~~~~~~~
In file included from edit.h:36:0,
/usr/include/ncursesw/curses.h:924:28: note: previous declaration of 'int find_pair(int, int)'
extern NCURSES_EXPORT(int) find_pair (int, int);
^~~~~~~~~
Makefile:1299: recipe for target 'color.o' failed
make[3]: *** [color.o] Error 1
ncurses was extended with new symbols, some of which conflict with "le"
internal names. So either ncurses should be fixed not to export these
symbols by default or le should be fixed to rename its identifiers.

--
Alexander.
Thomas Dickey
2017-07-17 09:40:01 UTC
Reply
Permalink
Raw Message
Post by Alexander V. Lukyanov
Source: le
Version: 1.16.3-1
Severity: serious
Tags: buster sid
https://tests.reproducible-builds.org/debian/rb-pkg/unstable/amd64/le.html
...
gcc -DHAVE_CONFIG_H -I. -I../lib -I../lib -I../lib -I/usr/include/ncursesw -Wdate-time -D_FORTIFY_SOURCE=2 -g -O2 -fstack-protector-strong -Wformat -Werror=format-security -Wall -Wwrite-strings -Woverloaded-virtual -fno-exceptions -fno-rtti -fno-implement-inlines -c -o color.o color.cc
color.cc:36:12: error: 'int find_pair(int, int)' was declared 'extern' and later 'static' [-fpermissive]
static int find_pair(int fg,int bg)
^~~~~~~~~
In file included from edit.h:36:0,
/usr/include/ncursesw/curses.h:924:28: note: previous declaration of 'int find_pair(int, int)'
extern NCURSES_EXPORT(int) find_pair (int, int);
^~~~~~~~~
Makefile:1299: recipe for target 'color.o' failed
make[3]: *** [color.o] Error 1
ncurses was extended with new symbols, some of which conflict with "le"
internal names. So either ncurses should be fixed not to export these
symbols by default or le should be fixed to rename its identifiers.
hmm - not that I'm oblivious to the problem, but (for example) a quick
check on an alternate name "find_color_pair" finds existing usage too.
This problem will come up since there's no namespaces in C (not that
C++ is free from the problem).

So... I could look for a rarely-used name (which still gives the same
connotations), or the couple of applications using ncurses could be
modified.
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Thomas Dickey
2017-07-22 14:50:02 UTC
Reply
Permalink
Raw Message
Source: le
Version: 1.16.3-1
Severity: serious
Tags: buster sid
https://tests.reproducible-builds.org/debian/rb-pkg/unstable/amd64/le.html
This is a problem with "le". I'm reassigning to that program, along with
a patch to fix the issues I found.
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Thomas Dickey
2017-07-22 15:00:01 UTC
Reply
Permalink
Raw Message
To ensure that I made a correct fix, I test-compiled le-1.16.3 and ran it.
Doing that, I noticed some additional issues (partly because I did not
override the makefile's C++ variables, but that was just as well, since
it prompted me to do the extra fixes):

a) renamed the private symbol find_pair to find_or_init_pair
b) include unistd.h to get prototype for write()
c) add a cast to fix a signed/unsigned compiler warning
d) add (to help with running valgrind) the ExitProgram macro
e) fix a different fail-to-build with the opaque TERMTYPE

Having done that, in a quick check the menus came up, and valgrind
had only reported an issue with linux_process_key() which I suppose
Alexander is familiar with.

There are still more than a hundred compiler warnings.
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Alexander V. Lukyanov
2017-07-25 07:10:01 UTC
Reply
Permalink
Raw Message
Post by Thomas Dickey
c) add a cast to fix a signed/unsigned compiler warning
I will check if a newer/better version of regex.c if available in emacs (it was taken from there).
Post by Thomas Dickey
d) add (to help with running valgrind) the ExitProgram macro
Where is HAVE__NC_FREE_AND_EXIT defined?
Post by Thomas Dickey
e) fix a different fail-to-build with the opaque TERMTYPE
I don't see how these lines are equivalent:

- TERMTYPE *tp = &cur_term->type;
+ TERMTYPE *tp = (TERMTYPE *)(&cur_term);

Can you elaborate?

--
Alexander.
Thomas Dickey
2017-07-25 09:10:01 UTC
Reply
Permalink
Raw Message
Post by Alexander V. Lukyanov
Post by Thomas Dickey
c) add a cast to fix a signed/unsigned compiler warning
I will check if a newer/better version of regex.c if available in emacs (it was taken from there).
Post by Thomas Dickey
d) add (to help with running valgrind) the ExitProgram macro
Where is HAVE__NC_FREE_AND_EXIT defined?
It would be defined if you had (an optional) configure check for
_nc_free_and_exit.

I do this for example when debugging ncurses applications and want to check
for memory leaks. But see

http://invisible-island.net/ncurses/ncurses.faq.html#config_leaks
Post by Alexander V. Lukyanov
Post by Thomas Dickey
e) fix a different fail-to-build with the opaque TERMTYPE
- TERMTYPE *tp = &cur_term->type;
+ TERMTYPE *tp = (TERMTYPE *)(&cur_term);
They're the same because the first member of TERMINAL happens to be
a TERMTYPE, and since TERMINAL is opaque in current code (so you
cannot refer to the "type" member any longer).
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Thomas Dickey
2017-07-26 09:30:01 UTC
Reply
Permalink
Raw Message
Post by Thomas Dickey
Post by Alexander V. Lukyanov
Post by Thomas Dickey
e) fix a different fail-to-build with the opaque TERMTYPE
- TERMTYPE *tp = &cur_term->type;
+ TERMTYPE *tp = (TERMTYPE *)(&cur_term);
They're the same because the first member of TERMINAL happens to be
a TERMTYPE, and since TERMINAL is opaque in current code (so you
cannot refer to the "type" member any longer).
But shouln't it be (TERMTYPE *)(cur_term) ?
hmm - I agree that doesn't look right (the problem with casts).
I'm surprised it worked.
Is there a new function or macro to get current TERMTYPE, so that I could
use it if available?
Not syntactically the same. This is what I use in term.h:

/* The cast works because TERMTYPE is the first data in TERMINAL */
#define CUR ((TERMTYPE *)(cur_term))->
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Thomas Dickey
2017-07-30 22:00:01 UTC
Reply
Permalink
Raw Message
Post by Thomas Dickey
Post by Thomas Dickey
Post by Alexander V. Lukyanov
Post by Thomas Dickey
e) fix a different fail-to-build with the opaque TERMTYPE
- TERMTYPE *tp = &cur_term->type;
+ TERMTYPE *tp = (TERMTYPE *)(&cur_term);
They're the same because the first member of TERMINAL happens to be
a TERMTYPE, and since TERMINAL is opaque in current code (so you
cannot refer to the "type" member any longer).
But shouln't it be (TERMTYPE *)(cur_term) ?
hmm - I agree that doesn't look right (the problem with casts).
I'm surprised it worked.
attaching an improved patch (with some additional compiler-warning fixes)

sadly enough, either way the cast gives no warnings, and the program
appears to run.

also attaching a copy of valgrind log from running the program with
the _nc_free_and_exit function turned on (just from opening a file,
looking at some menus and quitting - ymmv).
Post by Thomas Dickey
Is there a new function or macro to get current TERMTYPE, so that I could
use it if available?
/* The cast works because TERMTYPE is the first data in TERMINAL */
#define CUR ((TERMTYPE *)(cur_term))->
I'm reluctant to add a new function for accessing things that "should"
just work: the definitions in term.h are the defined interface, and
providing a function to access those in a different way doesn't seem
a good solution.

Of course, if that structure hadn't happened to be first in TERMINAL,
I'd have had to provide a function, just to keep TERMINAL opaque.

Since I'm keeping binary compatibility (aside from preventing applications
from knowing how large TERMINAL is), it's good enough for now.
--
Thomas E. Dickey <***@invisible-island.net>
http://invisible-island.net
ftp://invisible-island.net
Raphael Geissert
2017-08-18 10:50:02 UTC
Reply
Permalink
Raw Message
Alexander,

Do you plan to make a new release with the fixes? or should I grab the
patches from github?

I'd like to fix this some time soon to get le back in testing.

Thanks in advance.

Cheers,
--
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net
Alexander V. Lukyanov
2017-08-21 07:30:02 UTC
Reply
Permalink
Raw Message
Post by Raphael Geissert
Do you plan to make a new release with the fixes? or should I grab the
patches from github?
I'm going to release next version soon. I'm only waiting for a feedback
from a NetBSD user (compilation problem).

--
Alexander.
Alexander V. Lukyanov
2017-08-23 13:10:01 UTC
Reply
Permalink
Raw Message
Post by Raphael Geissert
Do you plan to make a new release with the fixes? or should I grab the
patches from github?
1.16.5 has been released.

--
Alexander.
Raphael Geissert
2017-08-23 14:40:02 UTC
Reply
Permalink
Raw Message
Post by Alexander V. Lukyanov
Post by Raphael Geissert
Do you plan to make a new release with the fixes? or should I grab the
patches from github?
1.16.5 has been released.
Awesome, thanks. I'll take care of the upload.

Cheers,
--
Raphael Geissert - Debian Developer
www.debian.org - get.debian.net
Loading...