Mirror of the gdb-patches mailing list
 help / color / mirror / Atom feed
* Fix tui compilation with Solaris libcurses (PR tui/21482)
@ 2017-05-18  8:56 Rainer Orth
  2017-05-18 13:19 ` Pedro Alves
  0 siblings, 1 reply; 9+ messages in thread
From: Rainer Orth @ 2017-05-18  8:56 UTC (permalink / raw)
  To: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 3709 bytes --]

On both mainline and the 8.0 branch, gdb compilation fails on Solaris 10
with the native libcurses:

* Initially, compilation failed like this:

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:2
6,
                 from /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c
:31:
/vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c: In function ‘CORE_A
DDR tui_disassemble(gdbarch*, tui_asm_line*, CORE_ADDR, int)’:
/vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c:71:19: error: ‘class
 string_file’ has no member named ‘wclear’; did you mean ‘clear’?
       gdb_dis_out.clear ();
                   ^
/vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c:78:19: error: ‘class
 string_file’ has no member named ‘wclear’; did you mean ‘clear’?
       gdb_dis_out.clear ();
                   ^
make[2]: *** [Makefile:1927: tui-disasm.o] Error 1

  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.*.

* 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).

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).

Ok for mainline and 8.0 branch?

	Rainer

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


2017-05-17  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	PR tui/21482
	* configure.ac <solaris2.*> (NOMACROS): Define.
	* configure: Regenerate.
	* config.in: Regenerate.

	* tui/tui-windata.c (tui_erase_data_content): Cast last mvwaddstr
	to char *.
	* tui/tui-wingeneral.c (box_win): Likewise.
	* tui/tui-winsource.c (tui_erase_source_content): Likewise.
	(tui_show_source_line): Likewise.
	(tui_show_exec_info_content): Likewise.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-gdb-tui-curses.patch --]
[-- Type: text/x-patch, Size: 2772 bytes --]

# HG changeset patch
# Parent  bfb43bbf7ca9808a9e660c111b68eef77999e76c
Fix tui compilation with Solaris libcurses (PR tui/21482)

diff --git a/gdb/configure.ac b/gdb/configure.ac
--- a/gdb/configure.ac
+++ b/gdb/configure.ac
@@ -1312,6 +1312,14 @@ case $host_os in
    Solaris 2.[789] when using GCC. ])
     fi ;;
 esac
+# On Solaris, we need to define NOMACROS so the native <curses.h> doesn't
+# define clear which interferes with the clear member of class string_file.
+case $host_os in
+  solaris2.*)
+    AC_DEFINE(NOMACROS, 1,
+      [Define to 1 to avoid <curses.h> defining clear which interferes with class string_file. ])
+    ;;
+esac
 AC_CHECK_HEADERS(curses.h cursesX.h ncurses.h ncurses/ncurses.h ncurses/term.h)
 AC_CHECK_HEADERS(term.h, [], [],
 [#if HAVE_CURSES_H
diff --git a/gdb/tui/tui-windata.c b/gdb/tui/tui-windata.c
--- a/gdb/tui/tui-windata.c
+++ b/gdb/tui/tui-windata.c
@@ -117,7 +117,7 @@ tui_erase_data_content (const char *prom
       mvwaddstr (TUI_DATA_WIN->generic.handle,
 		 (TUI_DATA_WIN->generic.height / 2),
 		 x_pos,
-		 prompt);
+		 (char *) prompt);
     }
   wrefresh (TUI_DATA_WIN->generic.handle);
 }
diff --git a/gdb/tui/tui-wingeneral.c b/gdb/tui/tui-wingeneral.c
--- a/gdb/tui/tui-wingeneral.c
+++ b/gdb/tui/tui-wingeneral.c
@@ -101,7 +101,7 @@ box_win (struct tui_gen_win_info *win_in
       box (win, tui_border_vline, tui_border_hline);
 #endif
       if (win_info->title)
-        mvwaddstr (win, 0, 3, win_info->title);
+        mvwaddstr (win, 0, 3, (char *) win_info->title);
       wattroff (win, attrs);
     }
 }
diff --git a/gdb/tui/tui-winsource.c b/gdb/tui/tui-winsource.c
--- a/gdb/tui/tui-winsource.c
+++ b/gdb/tui/tui-winsource.c
@@ -254,7 +254,7 @@ tui_erase_source_content (struct tui_win
 	  mvwaddstr (win_info->generic.handle,
 		     (win_info->generic.height / 2),
 		     x_pos,
-		     no_src_str);
+		     (char *) no_src_str);
 
 	  /* elz: Added this function call to set the real contents of
 	     the window to what is on the screen, so that later calls
@@ -280,7 +280,7 @@ tui_show_source_line (struct tui_win_inf
     wattron (win_info->generic.handle, A_STANDOUT);
 
   mvwaddstr (win_info->generic.handle, lineno, 1,
-             line->which_element.source.line);
+             (char *) line->which_element.source.line);
   if (line->which_element.source.is_exec_point)
     wattroff (win_info->generic.handle, A_STANDOUT);
 
@@ -565,7 +565,8 @@ tui_show_exec_info_content (struct tui_w
     mvwaddstr (exec_info->handle,
 	       cur_line,
 	       0,
-	       exec_info->content[cur_line - 1]->which_element.simple_string);
+	       (char *) exec_info->content[cur_line - 1]
+			  ->which_element.simple_string);
   tui_refresh_win (exec_info);
   exec_info->content_in_use = TRUE;
 }

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-18  8:56 Fix tui compilation with Solaris libcurses (PR tui/21482) Rainer Orth
@ 2017-05-18 13:19 ` Pedro Alves
  2017-05-18 13:36   ` Rainer Orth
  0 siblings, 1 reply; 9+ messages in thread
From: Pedro Alves @ 2017-05-18 13:19 UTC (permalink / raw)
  To: Rainer Orth, gdb-patches

On 05/18/2017 09:56 AM, Rainer Orth wrote:
> On both mainline and the 8.0 branch, gdb compilation fails on Solaris 10
> with the native libcurses:
> 
> * Initially, compilation failed like this:
> 
> 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:2
> 6,
>                  from /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c
> :31:
> /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c: In function ‘CORE_A
> DDR tui_disassemble(gdbarch*, tui_asm_line*, CORE_ADDR, int)’:
> /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c:71:19: error: ‘class
>  string_file’ has no member named ‘wclear’; did you mean ‘clear’?
>        gdb_dis_out.clear ();
>                    ^
> /vol/src/gnu/gdb/gdb-8.0-branch/local/gdb/tui/tui-disasm.c:78:19: error: ‘class
>  string_file’ has no member named ‘wclear’; did you mean ‘clear’?
>        gdb_dis_out.clear ();
>                    ^
> make[2]: *** [Makefile:1927: tui-disasm.o] Error 1
> 
>   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
checks NCURSES_NOMACROS instead of NOMACROS, we could define that too.

> 
> * 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.

> 
> 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).

> 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>.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-18 13:19 ` Pedro Alves
@ 2017-05-18 13:36   ` Rainer Orth
  2017-05-19 12:50     ` Rainer Orth
  0 siblings, 1 reply; 9+ messages in thread
From: Rainer Orth @ 2017-05-18 13:36 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-18 13:36   ` Rainer Orth
@ 2017-05-19 12:50     ` Rainer Orth
  2017-05-19 12:52       ` Pedro Alves
  2017-05-19 13:20       ` Eli Zaretskii
  0 siblings, 2 replies; 9+ messages in thread
From: Rainer Orth @ 2017-05-19 12:50 UTC (permalink / raw)
  To: Pedro Alves; +Cc: gdb-patches

[-- Attachment #1: Type: text/plain, Size: 1242 bytes --]

Hi Pedro,

>>> 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.

I've checked in the cast part now.  Here's the NOMACROS part for
gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
amd64-pc-solaris2.12 (ncurses).  Ok too?

Interestingly, with that patch the previous link failure on Solaris
11/12 (missing wattr_on/wattr_off) is gone, too.  This seems to happen
because <ncurses/ncurses.h> no longer defines

/usr/include/ncurses/ncurses.h:#define wattron(win,at)          wattr_on(win, NCURSES_CAST(attr_t, at), NULL)
/usr/include/ncurses/ncurses.h:#define attr_on(a,o)             wattr_on(stdscr,a,o)

and the references to wattron can be satisfied from libcurses just as
from libncurses.

Thanks.
        Rainer

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


2017-05-19  Rainer Orth  <ro@CeBiTec.Uni-Bielefeld.DE>

	PR tui/21482
	* gdb_curses.h (NOMACROS): Define.
	(NCURSES_NOMACROS): Define.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: sol2-gdb-tui-curses-clear.patch --]
[-- Type: text/x-patch, Size: 693 bytes --]

# HG changeset patch
# Parent  32840d58190409875fc9d8590fcb97eafaaeeca3
Fix tui compilation with Solaris libcurses: clear define (PR tui/21482)

diff --git a/gdb/gdb_curses.h b/gdb/gdb_curses.h
--- a/gdb/gdb_curses.h
+++ b/gdb/gdb_curses.h
@@ -32,6 +32,13 @@
 #undef KEY_EVENT
 #endif
 
+/* On Solaris and probably other SysVr4 derived systems, we need to define
+   NOMACROS so the native <curses.h> doesn't define clear which interferes
+   with the clear member of class string_file.  ncurses potentially has a
+   similar problem and fix.  */
+#define NOMACROS
+#define NCURSES_NOMACROS
+
 #if defined (HAVE_NCURSES_NCURSES_H)
 #include <ncurses/ncurses.h>
 #elif defined (HAVE_NCURSES_H)

^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-19 12:50     ` Rainer Orth
@ 2017-05-19 12:52       ` Pedro Alves
  2017-05-19 13:20       ` Eli Zaretskii
  1 sibling, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2017-05-19 12:52 UTC (permalink / raw)
  To: Rainer Orth; +Cc: gdb-patches

On 05/19/2017 01:50 PM, Rainer Orth wrote:
> Hi Pedro,
> 
>>>> 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.
> 
> I've checked in the cast part now.  Here's the NOMACROS part for
> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
> amd64-pc-solaris2.12 (ncurses).  Ok too?

OK.

> 
> Interestingly, with that patch the previous link failure on Solaris
> 11/12 (missing wattr_on/wattr_off) is gone, too.  This seems to happen
> because <ncurses/ncurses.h> no longer defines
> 
> /usr/include/ncurses/ncurses.h:#define wattron(win,at)          wattr_on(win, NCURSES_CAST(attr_t, at), NULL)
> /usr/include/ncurses/ncurses.h:#define attr_on(a,o)             wattr_on(stdscr,a,o)
> 
> and the references to wattron can be satisfied from libcurses just as
> from libncurses.

Eh, great.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  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:39         ` Pedro Alves
  1 sibling, 2 replies; 9+ messages in thread
From: Eli Zaretskii @ 2017-05-19 13:20 UTC (permalink / raw)
  To: Rainer Orth; +Cc: palves, gdb-patches

> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
> Cc: gdb-patches@sourceware.org
> Date: Fri, 19 May 2017 14:50:06 +0200
> 
> I've checked in the cast part now.  Here's the NOMACROS part for
> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
> amd64-pc-solaris2.12 (ncurses).  Ok too?

I think this should be guarded by some OS-specific macro, so as not to
affect other platforms, where the original problem doesn't exist.  (I
see 6 instances of these macros being tested in my ncurses headers,
and I'm not on Solaris.)  Who knows what new problems this could cause?

Thanks.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  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
  1 sibling, 1 reply; 9+ messages in thread
From: Rainer Orth @ 2017-05-19 13:26 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: palves, gdb-patches

Hi Eli,

>> I've checked in the cast part now.  Here's the NOMACROS part for
>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>> amd64-pc-solaris2.12 (ncurses).  Ok too?
>
> I think this should be guarded by some OS-specific macro, so as not to
> affect other platforms, where the original problem doesn't exist.  (I
> see 6 instances of these macros being tested in my ncurses headers,
> and I'm not on Solaris.)  Who knows what new problems this could cause?

that's what I had done initially (via configure.ac for solaris2.* only),
but Pedro suggested to do it unconditionally since some other targets
(AIX notably) seem to be having the same problem.

	Rainer

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


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-19 13:20       ` Eli Zaretskii
  2017-05-19 13:26         ` Rainer Orth
@ 2017-05-19 13:39         ` Pedro Alves
  1 sibling, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2017-05-19 13:39 UTC (permalink / raw)
  To: Eli Zaretskii, Rainer Orth; +Cc: gdb-patches

On 05/19/2017 02:20 PM, Eli Zaretskii wrote:
>> From: Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE>
>> Cc: gdb-patches@sourceware.org
>> Date: Fri, 19 May 2017 14:50:06 +0200
>>
>> I've checked in the cast part now.  Here's the NOMACROS part for
>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>> amd64-pc-solaris2.12 (ncurses).  Ok too?
> 
> I think this should be guarded by some OS-specific macro, so as not to
> affect other platforms, where the original problem doesn't exist. 

The problem exists, we just hadn't tripped on it yet.  
Googling around we see people running into curses defining "move" as
macro for instance, which conflicts with std::move.  

So I'd rather take the opposite approach.  GDB must be able to compile
with  NOMACROS defined on some hosts, so defining it everywhere makes
gdb behave the same everywhere and reduces the testing axis.  A patch
introducing a problem for Solaris or any host using the same curses
will be quickly exposed on any host.

> (I
> see 6 instances of these macros being tested in my ncurses headers,
> and I'm not on Solaris.)  Who knows what new problems this could cause?

I expect it'll fix more problems than create them.  Defining macros without
some sort of namespace prefix in C++ is a sure recipe for problems.
NOMACROS was surely invented to avoid these problems.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: Fix tui compilation with Solaris libcurses (PR tui/21482)
  2017-05-19 13:26         ` Rainer Orth
@ 2017-05-19 13:43           ` Pedro Alves
  0 siblings, 0 replies; 9+ messages in thread
From: Pedro Alves @ 2017-05-19 13:43 UTC (permalink / raw)
  To: Rainer Orth, Eli Zaretskii; +Cc: gdb-patches

On 05/19/2017 02:26 PM, Rainer Orth wrote:
> Hi Eli,
> 
>>> I've checked in the cast part now.  Here's the NOMACROS part for
>>> gdb_curses.h.  Tested as before on sparcv9-sun-solaris2.10 (curses) and
>>> amd64-pc-solaris2.12 (ncurses).  Ok too?
>>
>> I think this should be guarded by some OS-specific macro, so as not to
>> affect other platforms, where the original problem doesn't exist.  (I
>> see 6 instances of these macros being tested in my ncurses headers,
>> and I'm not on Solaris.)  Who knows what new problems this could cause?
> 
> that's what I had done initially (via configure.ac for solaris2.* only),
> but Pedro suggested to do it unconditionally since some other targets
> (AIX notably) seem to be having the same problem.

Yes, and it's not host specific, but really curses-implementation
specific.  On the same host you may compile against different versions
of curses (BSD curses, ncurses, pdcurses, etc.).  I don't see any
benefit to complicate things when we have no evidence that telling
curses to avoid defining its symbols as macros causes problems.

Thanks,
Pedro Alves


^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2017-05-19 13:43 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2017-05-18  8:56 Fix tui compilation with Solaris libcurses (PR tui/21482) Rainer Orth
2017-05-18 13:19 ` Pedro Alves
2017-05-18 13:36   ` Rainer Orth
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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox