Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
To: Pedro Alves <palves@redhat.com>
Cc: gdb-patches@sourceware.org
Subject: Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
Date: Thu, 18 May 2017 13:36:00 -0000	[thread overview]
Message-ID: <yddr2zmz3qc.fsf@CeBiTec.Uni-Bielefeld.DE> (raw)
In-Reply-To: <32521e83-00b5-e2a8-faff-03b5407cfc67@redhat.com> (Pedro Alves's	message of "Thu, 18 May 2017 14:19:40 +0100")

Hi Pedro,

>>   It turned out this happens because <curses.h> has
>> 
>> #define clear()         wclear(stdscr)
>> 
>>   This can be avoided by defining NOMACROS, which the patch below does
>>   for solaris2.*.
>
> We already handle some curses warts in gdb_curses.h (and then
> include that header instead everywhere).  I think this could go there,
> even unconditionally.  (This is more about curses implementation
> than OS strictly speaking.  Googling around finds hits for that
> macro in the AIX curses.h header, for example.).  Looks like ncurses

probably of SysVr4 origin ultimately: at least I already found it in
those sources.

> checks NCURSES_NOMACROS instead of NOMACROS, we could define that too.

Makes sense (unless this creates problems of its own ;-).

>> * Even with this workaround, compilation fails in gdb/tui for several
>>   instances of the same problem:
>> 
>> /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-winsource.c: In function ‘void tui_erase_source_content(tui_win_info*, int)’:
>> /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-winsource.c:257:18: error: invalid conversion from ‘const char*’ to ‘char*’ [-fpermissive]
>>         no_src_str);
>>                   ^
>> In file included from /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/gdb_curses.h:42:0,
>>                  from /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-data.h:26,
>>                  from /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-winsource.c:33:
>> /vol/gcc-7/lib/gcc/sparc-sun-solaris2.10/7.1.0/include-fixed/curses.h:699:12: note:   initializing argument 4 of ‘int mvwaddstr(WINDOW*, int, int, char*)’
>>  extern int mvwaddstr(WINDOW *, int, int, char *);
>>             ^~~~~~~~~
>> make[2]: *** [Makefile:1927: tui-winsource.o] Error 1
>> 
>>   Unlike ncurses, <curses.h> declares 
>> 
>> extern int mvwaddstr(WINDOW *, int, int, char *);
>> 
>>   i.e. the last arg is char *, not const char *.
>> 
>>   The patch fixes this by casting the last arg to mvwaddstr to char *,
>>   as was recently done on mainline in a newterm() call (the only
>>   difference between 8.0 and mainline gdb/tui).
>
> That's fine with me.
>
> Looking at:
>
>  https://docs.oracle.com/cd/E19455-01/806-0629/6j9vjco9i/index.html
>
> I see that this affects several APIs, so nicer would be to
> fix this centrally in gdb_curses.h like we fix e.g.,
> PyObject_GetAttrString in python/python-internal.h.  But that can
> be for another day.

This way you centralize the knowledge why you are doing this in once
header rather than several calls/casts.  I guess we'll cross that bridge
once another function causes similar trouble.

>> With those changes, gdb on the 8.0 branch compiles cleanly on
>> sparcv9-sun-solaris2.10 with native curses and amd64-pc-solaris2.12
>> with bundled ncurses (well, almost: on Solaris 12 ncurses in
>> /usr/include is used, but gdb linked against -lcurses which fails:
>> 
>> Undefined                       first referenced
>>  symbol                             in file
>> wattr_on                            tui-wingeneral.o
>> wattr_off                           tui-wingeneral.o
>> 
>> but that's a different and preexisting problem).
>
> These two problems would be better pushed as two separate patches
> (with the rationales given above as separate commit logs).

Fine with me.  I'll look at the ncurses header vs. libcurses issue
later: so far I've just been lazy and manually linked gdb if I hit it.

>> Ok for mainline and 8.0 branch?
>
> The cast bits are OK.  I'd like to hear your opinion on
> moving the NOMACROS define to gdb_curses.h, before including
> <curses.h>.

The move makes sense to me: I just wasn't aware of that file.  I'll
prepare a separate patch.

Thanks.
        Rainer

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University


  reply	other threads:[~2017-05-18 13:36 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-05-18  8:56 Rainer Orth
2017-05-18 13:19 ` Pedro Alves
2017-05-18 13:36   ` Rainer Orth [this message]
2017-05-19 12:50     ` Rainer Orth
2017-05-19 12:52       ` Pedro Alves
2017-05-19 13:20       ` Eli Zaretskii
2017-05-19 13:26         ` Rainer Orth
2017-05-19 13:43           ` Pedro Alves
2017-05-19 13:39         ` Pedro Alves

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=yddr2zmz3qc.fsf@CeBiTec.Uni-Bielefeld.DE \
    --to=ro@cebitec.uni-bielefeld.de \
    --cc=gdb-patches@sourceware.org \
    --cc=palves@redhat.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox