[RFA] Fix compilation failure on cygwin due to ncurses macro.

[RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

  I currently get compilation failures
on cygwin (old 1.5 version)
  in tui/tui-regs.c
on wstandout/wstandend ncurses functions.


Using --save-temps, I found out that
this is because wstandout is translated into:

static void
tui_display_register (struct tui_data_element *data,
                      struct tui_gen_win_info *win_info)
{
  if (win_info->handle != (WINDOW *) ((void *)0))
    {
      int i;

      if (data->highlight)
 (((win_info->handle)->_attrs = (((1UL) << ((8) + 8))),
(int)((win_info->handle)
->_attrs)));
but the second part is not used.
leading to an unused value warning transformed into an error.
 
  The wstandout macro comes from /usr/include/ncurses/ncurses.h
version is "5.7" patch "20091114"
 


Casting the calls of wstandout and wstandend to (void) fixed the compilation
error
for me.


Is this patch OK?

Pierre Muller
Pascal language support maintainer for GDB


2010-04-13  Pierre Muller  <muller@ics.u-strasbg.fr>

	Suppress unused value warning during compilation .
	* tui/tui-regs.c (tui_display_register): Cast wstandout and
wstandend
	calls to (void).
	* tui/tui-stack.c (tui_show_locator_content): Likewise.

Index: tui/tui-regs.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-regs.c,v
retrieving revision 1.36
diff -u -p -r1.36 tui-regs.c
--- tui/tui-regs.c	1 Jan 2010 07:32:07 -0000	1.36
+++ tui/tui-regs.c	13 Apr 2010 08:50:56 -0000
@@ -541,7 +541,7 @@ tui_display_register (struct tui_data_el
       int i;
 
       if (data->highlight)
-	wstandout (win_info->handle);
+	(void) wstandout (win_info->handle);
       
       wmove (win_info->handle, 0, 0);
       for (i = 1; i < win_info->width; i++)
@@ -551,7 +551,7 @@ tui_display_register (struct tui_data_el
         waddstr (win_info->handle, data->content);
 
       if (data->highlight)
-	wstandend (win_info->handle);
+	(void) wstandend (win_info->handle);
       tui_refresh_win (win_info);
     }
 }
Index: tui/tui-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-stack.c,v
retrieving revision 1.34
diff -u -p -r1.34 tui-stack.c
--- tui/tui-stack.c	1 Jan 2010 07:32:07 -0000	1.34
+++ tui/tui-stack.c	13 Apr 2010 08:50:56 -0000
@@ -256,10 +256,10 @@ tui_show_locator_content (void)
 
       string = tui_make_status_line (&element->which_element.locator);
       wmove (locator->handle, 0, 0);
-      wstandout (locator->handle);
+      (void) wstandout (locator->handle);
       waddstr (locator->handle, string);
       wclrtoeol (locator->handle);
-      wstandend (locator->handle);
+      (void) wstandend (locator->handle);
       tui_refresh_win (locator);
       wmove (locator->handle, 0, 0);
       xfree (string);

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pedro Alves]

On Tuesday 13 April 2010 09:56:17, Pierre Muller wrote:
> Is this patch OK?

Sure.

> 2010-04-13  Pierre Muller  <muller@ics.u-strasbg.fr>
> 
>         Suppress unused value warning during compilation .

                                                          ^

spurious space.

>         * tui/tui-regs.c (tui_display_register): Cast wstandout and
> wstandend
>         calls to (void).
>         * tui/tui-stack.c (tui_show_locator_content): Likewise.


FYI: the type is void, not (void).  ()'s are part of the
C cast syntax.

-- 
Pedro Alves

RE: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Pedro Alves
> Envoyé : Tuesday, April 13, 2010 1:38 PM
> À : gdb-patches@sourceware.org
> Cc : Pierre Muller
> Objet : Re: [RFA] Fix compilation failure on cygwin due to ncurses
> macro.
> 
> On Tuesday 13 April 2010 09:56:17, Pierre Muller wrote:
> > Is this patch OK?
> 
> Sure.
Thanks, Pedro.

Committed with your corrections.

Pierre

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Joel Brobecker]

> Committed with your corrections.

Just a request: Can you add a comment next to all the casts explaining
why the cast is there? It's important so that someone seeing the cast
does not scratch his head for too long...

Thank you,
-- 
Joel

RE: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Tuesday, April 13, 2010 5:07 PM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix compilation failure on cygwin due to ncurses
> macro.
> 
> > Committed with your corrections.
> 
> Just a request: Can you add a comment next to all the casts explaining
> why the cast is there? It's important so that someone seeing the cast
> does not scratch his head for too long...

Would this change be OK?
Any better description is equally OK for me...

Does it deserve a separate ChangeLog entry, or should I commit it
without ChangeLog entry?

Pierre

$ cvs diff -u -p tui/tui-regs.c tui/tui-stack.c
Index: tui/tui-regs.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-regs.c,v
retrieving revision 1.37
diff -u -p -r1.37 tui-regs.c
--- tui/tui-regs.c      13 Apr 2010 12:11:09 -0000      1.37
+++ tui/tui-regs.c      13 Apr 2010 15:14:58 -0000
@@ -541,6 +541,8 @@ tui_display_register (struct tui_data_el
       int i;

       if (data->highlight)
+      /* Typecast to void is needed because some ncurses macros expand to
code
+        generating a warning about an unused value otherwise.  */
        (void) wstandout (win_info->handle);

       wmove (win_info->handle, 0, 0);
@@ -551,6 +553,8 @@ tui_display_register (struct tui_data_el
         waddstr (win_info->handle, data->content);

       if (data->highlight)
+      /* Typecast to void is needed because some ncurses macros expand to
code
+        generating a warning about an unused value otherwise.  */
        (void) wstandend (win_info->handle);
       tui_refresh_win (win_info);
     }
Index: tui/tui-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-stack.c,v
retrieving revision 1.35
diff -u -p -r1.35 tui-stack.c
--- tui/tui-stack.c     13 Apr 2010 12:11:10 -0000      1.35
+++ tui/tui-stack.c     13 Apr 2010 15:14:58 -0000
@@ -256,9 +256,13 @@ tui_show_locator_content (void)

       string = tui_make_status_line (&element->which_element.locator);
       wmove (locator->handle, 0, 0);
+      /* Typecast to void is needed because some ncurses macros expand to
code
+        generating a warning about an unused value otherwise.  */
       (void) wstandout (locator->handle);
       waddstr (locator->handle, string);
       wclrtoeol (locator->handle);
+      /* Typecast to void is needed because some ncurses macros expand to
code
+        generating a warning about an unused value otherwise.  */
       (void) wstandend (locator->handle);
       tui_refresh_win (locator);
       wmove (locator->handle, 0, 0);

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Joel Brobecker]

> Would this change be OK?
> Any better description is equally OK for me...

How about the following? Please replace "unused-value" by what the
compiler actually complained about...

  /* We need to cast the return value of wstandout to void because
     older versions of the ncurses implementation (for instance,
     the version used with cygwin 1.5) expand to some code leading
     to an unused-value warning from the compiler.  */

-- 
Joel

RE: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Tuesday, April 13, 2010 5:26 PM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix compilation failure on cygwin due to ncurses
> macro.
> 
> > Would this change be OK?
> > Any better description is equally OK for me...
> 
> How about the following? Please replace "unused-value" by what the
> compiler actually complained about...
> 
>   /* We need to cast the return value of wstandout to void because
>      older versions of the ncurses implementation (for instance,
>      the version used with cygwin 1.5) expand to some code leading
>      to an unused-value warning from the compiler.  */

  Joel, the file I was referring to was generated from
`ncurses-5.7/include/ncurses.h.in'
which is the last official release of ncurses according to
GNU web site, so 'older versions" seems a bit early to me!
  In fact it is even worse than this:
the Cygwin distributed version is a patched version
from 2009/11/14 available at ftp://invisible-island.net/ncurses/5.7/
which means that it will probably be included in the next distribution.

Pierre  

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Joel Brobecker]

> >   /* We need to cast the return value of wstandout to void because
> >      older versions of the ncurses implementation (for instance,
> >      the version used with cygwin 1.5) expand to some code leading
> >      to an unused-value warning from the compiler.  */
> 
>   Joel, the file I was referring to was generated from
> `ncurses-5.7/include/ncurses.h.in'
> which is the last official release of ncurses according to
> GNU web site, so 'older versions" seems a bit early to me!

Oh... Can you make an amendment?

-- 
Joel

RE: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

What about this:
      /* We need to cast the return value of wstandout to void because
         in patches to release "5.7" of ncurses dated after 2009/05/16,
         the wstandout macro expands to code that generates a unused-value
         warning at compilation.  */

  
At least, this is what I understood from the patched ncurses NEWS:

20090516
       + work around antique BSD game's manipulation of stdscr, etc., versus
         SCREEN's copy of the pointer (Debian #528411).
       + add a cast to wattrset macro to avoid compiler warning when
comparing
         its result against ERR (adapted from patch by Matt Kraii, Debian
         #528374).

But I couldn't get the patch itself...
  
Pierre

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Wednesday, April 14, 2010 6:00 PM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix compilation failure on cygwin due to ncurses
> macro.
> 
> > >   /* We need to cast the return value of wstandout to void because
> > >      older versions of the ncurses implementation (for instance,
> > >      the version used with cygwin 1.5) expand to some code leading
> > >      to an unused-value warning from the compiler.  */
> >
> >   Joel, the file I was referring to was generated from
> > `ncurses-5.7/include/ncurses.h.in'
> > which is the last official release of ncurses according to
> > GNU web site, so 'older versions" seems a bit early to me!
> 
> Oh... Can you make an amendment?
> 
> --
> Joel

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Joel Brobecker]

>       /* We need to cast the return value of wstandout to void because
>          in patches to release "5.7" of ncurses dated after 2009/05/16,
>          the wstandout macro expands to code that generates a unused-value
>          warning at compilation.  */

I think this is going to be good enough. Just a question: Whose patches
are we referring to?

-- 
Joel

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pedro Alves]

On Wednesday 14 April 2010 17:29:19, Pierre Muller wrote:
> At least, this is what I understood from the patched ncurses NEWS:
> 
> 20090516
>        + work around antique BSD game's manipulation of stdscr, etc., versus
>          SCREEN's copy of the pointer (Debian #528411).
>        + add a cast to wattrset macro to avoid compiler warning when
> comparing
>          its result against ERR (adapted from patch by Matt Kraii, Debian
                                                                     ^^^^^^
>          #528374).
           ^^^^^^^
> 
> But I couldn't get the patch itself...

Googling for that helps.

 http://www.mail-archive.com/debian-bugs-closed@lists.debian.org/msg241443.html

Notice no comma operator there; perhaps that's what needed adjustment.

-- 
Pedro Alves

RE: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Pierre Muller]

 I finally found the whole series of patches at
http://ftp.ec-m.fr/pub/FreeBSD/ports/local-distfiles/rafan/ncurses/

  The separate NCURSES_CAST(int, (win)->attrs)
was introduced in 20090829 patch.
  According to NEWS, this is to solve Debian #542031.
 
Thus it should become:
      /* We need to cast the return value of wstandout to void because
         in 2009/08/29 patch to release "5.7" of ncurses library,
         the wstandout macro expands to code that generates a unused-value
         warning at compilation.  */

Is this correct?

Pierre

Re: [RFA] Fix compilation failure on cygwin due to ncurses macro.

[Joel Brobecker]

> Thus it should become:
>       /* We need to cast the return value of wstandout to void because
>          in 2009/08/29 patch to release "5.7" of ncurses library,
>          the wstandout macro expands to code that generates a unused-value
>          warning at compilation.  */
> 
> Is this correct?

Sounds right to me - just one nit: I think it's "an unused-value" rather
than "a unused...".

Suggestion (which you are free to ignore): Would it make sense to define
a new function or macro to have the cast (and therefore the comment) in
only one place?

-- 
Joel

[RFC] Use macro for void typecast necessary for ncurses library.

[Pierre Muller]

> -----Message d'origine-----
> De : gdb-patches-owner@sourceware.org [mailto:gdb-patches-
> owner@sourceware.org] De la part de Joel Brobecker
> Envoyé : Thursday, April 15, 2010 5:53 PM
> À : Pierre Muller
> Cc : 'Pedro Alves'; gdb-patches@sourceware.org
> Objet : Re: [RFA] Fix compilation failure on cygwin due to ncurses
> macro.
> 
> > Thus it should become:
> >       /* We need to cast the return value of wstandout to void
> because
> >          in 2009/08/29 patch to release "5.7" of ncurses library,
> >          the wstandout macro expands to code that generates a unused-
> value
> >          warning at compilation.  */
> >
> > Is this correct?
> 
> Sounds right to me - just one nit: I think it's "an unused-value"
> rather
> than "a unused...".
> 
> Suggestion (which you are free to ignore): Would it make sense to
> define
> a new function or macro to have the cast (and therefore the comment) in
> only one place?
  Here is an implementation of your idea.

  I am unsure about:
- the name we should give to the macro.
- should the macro have a parameter?
- should I mention that it would also apply to uses of wattrset?


2010-04-16  Pierre Muller  <muller@ics.u-strasbg.fr>

	* gdb_curses.h (CURSES_CAST_TO_VOID): New macro, with
	explaination about its use.
	* tui/tui-regs.c (tui_display_register): Use new macro.
	* tui/tui-stack.c (tui_show_locator_content): Likewise.

Index: gdb_curses.h
===================================================================
RCS file: /cvs/src/src/gdb/gdb_curses.h,v
retrieving revision 1.12
diff -u -p -r1.12 gdb_curses.h
--- gdb_curses.h	1 Jan 2010 07:31:32 -0000	1.12
+++ gdb_curses.h	15 Apr 2010 22:30:53 -0000
@@ -43,4 +43,12 @@
 extern int tgetnum (const char *);
 #endif
 
+
+/* We need to cast the return value of wstandout and wstandend to void
because
+   in 2009/08/29 patch to release "5.7" of ncurses library,
+   the wstandout/wstandend macros expands to code that generates an
unused-value
+   warning at compilation.  */
+
+#define CURSES_CAST_TO_VOID (void)
+
 #endif /* gdb_curses.h */
Index: tui/tui-regs.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-regs.c,v
retrieving revision 1.37
diff -u -p -r1.37 tui-regs.c
--- tui/tui-regs.c	13 Apr 2010 12:11:09 -0000	1.37
+++ tui/tui-regs.c	15 Apr 2010 22:30:54 -0000
@@ -541,7 +541,7 @@ tui_display_register (struct tui_data_el
       int i;
 
       if (data->highlight)
-	(void) wstandout (win_info->handle);
+	CURSES_CAST_TO_VOID wstandout (win_info->handle);
       
       wmove (win_info->handle, 0, 0);
       for (i = 1; i < win_info->width; i++)
@@ -551,7 +551,7 @@ tui_display_register (struct tui_data_el
         waddstr (win_info->handle, data->content);
 
       if (data->highlight)
-	(void) wstandend (win_info->handle);
+	CURSES_CAST_TO_VOID wstandend (win_info->handle);
       tui_refresh_win (win_info);
     }
 }
Index: tui/tui-stack.c
===================================================================
RCS file: /cvs/src/src/gdb/tui/tui-stack.c,v
retrieving revision 1.35
diff -u -p -r1.35 tui-stack.c
--- tui/tui-stack.c	13 Apr 2010 12:11:10 -0000	1.35
+++ tui/tui-stack.c	15 Apr 2010 22:30:54 -0000
@@ -256,10 +256,10 @@ tui_show_locator_content (void)
 
       string = tui_make_status_line (&element->which_element.locator);
       wmove (locator->handle, 0, 0);
-      (void) wstandout (locator->handle);
+      CURSES_CAST_TO_VOID wstandout (locator->handle);
       waddstr (locator->handle, string);
       wclrtoeol (locator->handle);
-      (void) wstandend (locator->handle);
+      CURSES_CAST_TO_VOID wstandend (locator->handle);
       tui_refresh_win (locator);
       wmove (locator->handle, 0, 0);
       xfree (string);

Re: [RFC] Use macro for void typecast necessary for ncurses library.

[Joel Brobecker]

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

> > Suggestion (which you are free to ignore): Would it make sense to
> > define
> > a new function or macro to have the cast (and therefore the comment) in
> > only one place?
>   Here is an implementation of your idea.

Argh - I didn't realize that two "routines" where involved. For some
reason, when I looked at the initial patch, I only noticed wstandout,
and I thought it was used 4 times.

For the record - my idea was to define a macro such as unchecked_wstandout
that contains the cast, but if we have to do it for at least 2 routines,
there is no real benefit.

I ended up checking in the attached patch.  Sorry about the screwup,
and thanks for giving it a try...

-- 
Joel

[-- Attachment #2: tui-cast-void.diff --]
[-- Type: text/x-diff, Size: 2617 bytes --]

commit e0737f6d628f1aa573cc2d1fd827be90b5d48a09
Author: Joel Brobecker <brobecker@adacore.com>
Date:   Thu Apr 15 21:32:52 2010 -0700

            * tui/tui-regs.c (tui_display_register): Add comment about
            a couple of casts.
            * tui/tui-stack.c (tui_show_locator_content): Ditto.

diff --git a/gdb/ChangeLog b/gdb/ChangeLog
index 0a8f5b1..7c2ec94 100644
--- a/gdb/ChangeLog
+++ b/gdb/ChangeLog
@@ -1,3 +1,9 @@
+2010-04-16  Joel Brobecker  <brobecker@adacore.com>
+
+	* tui/tui-regs.c (tui_display_register): Add comment about
+	a couple of casts.
+	* tui/tui-stack.c (tui_show_locator_content): Ditto.
+
 2010-04-15  Stan Shebs  <stan@codesourcery.com>
 
 	* frame.c: Include tracepoint.h.
diff --git a/gdb/tui/tui-regs.c b/gdb/tui/tui-regs.c
index 8cbd338..a339bea 100644
--- a/gdb/tui/tui-regs.c
+++ b/gdb/tui/tui-regs.c
@@ -541,6 +541,11 @@ tui_display_register (struct tui_data_element *data,
       int i;
 
       if (data->highlight)
+	/* We ignore the return value, casting it to void in order to avoid
+	   a compiler warning.  The warning itself was introduced by a patch
+	   to ncurses 5.7 dated 2009-08-29, changing this macro to expand
+	   to code that causes the compiler to generate an unused-value
+	   warning.  */
 	(void) wstandout (win_info->handle);
       
       wmove (win_info->handle, 0, 0);
@@ -551,6 +556,11 @@ tui_display_register (struct tui_data_element *data,
         waddstr (win_info->handle, data->content);
 
       if (data->highlight)
+	/* We ignore the return value, casting it to void in order to avoid
+	   a compiler warning.  The warning itself was introduced by a patch
+	   to ncurses 5.7 dated 2009-08-29, changing this macro to expand
+	   to code that causes the compiler to generate an unused-value
+	   warning.  */
 	(void) wstandend (win_info->handle);
       tui_refresh_win (win_info);
     }
diff --git a/gdb/tui/tui-stack.c b/gdb/tui/tui-stack.c
index 1630bea..d6b514d 100644
--- a/gdb/tui/tui-stack.c
+++ b/gdb/tui/tui-stack.c
@@ -256,6 +256,11 @@ tui_show_locator_content (void)
 
       string = tui_make_status_line (&element->which_element.locator);
       wmove (locator->handle, 0, 0);
+      /* We ignore the return value from wstandout and wstandend, casting
+	 them to void in order to avoid a compiler warning.  The warning
+	 itself was introduced by a patch to ncurses 5.7 dated 2009-08-29,
+	 changing these macro to expand to code that causes the compiler
+	 to generate an unused-value warning.  */
       (void) wstandout (locator->handle);
       waddstr (locator->handle, string);
       wclrtoeol (locator->handle);

Re: [RFC] Use macro for void typecast necessary for ncurses library.

[Tom Tromey]

>>>>> "Pierre" == Pierre Muller <pierre.muller@ics-cnrs.unistra.fr> writes:

Pierre>   I am unsure about:
Pierre> - the name we should give to the macro.
Pierre> - should the macro have a parameter?
Pierre> - should I mention that it would also apply to uses of wattrset?

If you are going to use a macro, then I would say it ought to have a
parameter.  I think the CURSES_CAST_TO_VOID thing is less readable than
a plain (void) cast.

Tom