Minor src/xmenu.c change for older GCC versions

Minor src/xmenu.c change for older GCC versions

[Giorgos Keramidas]

On FreeBSD 4.X we use GCC 2.95.4 20020320 [FreeBSD] as the system
compiler.  The src/xmenu.c source triggers an error when compiled with
this version of GCC, so we had to make the following local change
as part of the editors/emacs-devel port of Emacs 22.X to FreeBSD:

%%%
diff -r e9c18eba38ea -r 355e6a7d786b src/xmenu.c
--- a/src/xmenu.c	Fri Oct 27 15:45:51 2006 +0000
+++ b/src/xmenu.c	Sat Oct 28 01:49:45 2006 +0300
@@ -1395,9 +1395,10 @@ If FRAME is nil or not given, use the se
      Lisp_Object frame;
 {
   GtkWidget *menubar;
+  FRAME_PTR f;
+
   BLOCK_INPUT;
-  FRAME_PTR f = check_x_frame (frame);
-
+  f = check_x_frame (frame);
   if (FRAME_EXTERNAL_MENU_BAR (f))
     set_frame_menubar (f, 0, 1);
 
%%%

Apparently, gcc 2.95.4 does not accept the FRAME_PTR declaration *after*
BLOCK_INPUT, because it expects all declarations to come before
BLOCK_INPUT expands to executable parts.

Does this change look like something we can add to Emacs before pretest
gets released?  It's not really that intrusive, and it will reduce the
number of local patches we have to use to build Emacs 22.X on FreeBSD,
when it's officially released.

Regards,
Giorgos

Re: Minor src/xmenu.c change for older GCC versions

[Chong Yidong]

Giorgos Keramidas <keramida@ceid.upatras.gr> writes:

> On FreeBSD 4.X we use GCC 2.95.4 20020320 [FreeBSD] as the system
> compiler.  The src/xmenu.c source triggers an error when compiled with
> this version of GCC, so we had to make the following local change
> as part of the editors/emacs-devel port of Emacs 22.X to FreeBSD:
>
> %%%
> diff -r e9c18eba38ea -r 355e6a7d786b src/xmenu.c
> --- a/src/xmenu.c	Fri Oct 27 15:45:51 2006 +0000
> +++ b/src/xmenu.c	Sat Oct 28 01:49:45 2006 +0300
> @@ -1395,9 +1395,10 @@ If FRAME is nil or not given, use the se
>       Lisp_Object frame;
>  {
>    GtkWidget *menubar;
> +  FRAME_PTR f;
> +
>    BLOCK_INPUT;
> -  FRAME_PTR f = check_x_frame (frame);
> -
> +  f = check_x_frame (frame);
>    if (FRAME_EXTERNAL_MENU_BAR (f))
>      set_frame_menubar (f, 0, 1);

Checked in.  Thanks.

Re: Minor src/xmenu.c change for older GCC versions

[Richard Stallman]

    > +  FRAME_PTR f;
    > +
    >    BLOCK_INPUT;
    > -  FRAME_PTR f = check_x_frame (frame);
    > -
    > +  f = check_x_frame (frame);

Please add a comment explaining why this is necessary, to make sure
people don't take this out for elegance's sake.

Re: Minor src/xmenu.c change for older GCC versions

[Giorgos Keramidas]

On 2006-11-05 02:07, Richard Stallman <rms@gnu.org> wrote:
>     > +  FRAME_PTR f;
>     > +
>     >    BLOCK_INPUT;
>     > -  FRAME_PTR f = check_x_frame (frame);
>     > -
>     > +  f = check_x_frame (frame);
> 
> Please add a comment explaining why this is necessary, to make sure
> people don't take this out for elegance's sake.

Good point.  Sorry I didn't add a comment originally.  I thought
mentioning it in the commit log would be sufficient for now :)

Re: Minor src/xmenu.c change for older GCC versions

[Eli Zaretskii]

> Date: Sun, 5 Nov 2006 10:00:57 +0200
> From: Giorgos Keramidas <keramida@ceid.upatras.gr>
> Cc: Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
> > 
> > Please add a comment explaining why this is necessary, to make sure
> > people don't take this out for elegance's sake.
> 
> Good point.  Sorry I didn't add a comment originally.  I thought
> mentioning it in the commit log would be sufficient for now :)

It's actually the other way around: the ChangeLog entry should only
tell WHAT has been changed; the WHY should be in the comments to the
code.  See the GNU coding standards for more about this.

Re: Minor src/xmenu.c change for older GCC versions

[Giorgos Keramidas]

On 2006-11-05 14:06, Eli Zaretskii <eliz@gnu.org> wrote:
>>Date: Sun, 5 Nov 2006 10:00:57 +0200
>>From: Giorgos Keramidas <keramida@ceid.upatras.gr>
>>Cc: Chong Yidong <cyd@stupidchicken.com>, emacs-devel@gnu.org
>>>
>>> Please add a comment explaining why this is necessary, to make sure
>>> people don't take this out for elegance's sake.
>>
>> Good point.  Sorry I didn't add a comment originally.  I thought
>> mentioning it in the commit log would be sufficient for now :)
>
> It's actually the other way around: the ChangeLog entry should only
> tell WHAT has been changed; the WHY should be in the comments to the
> code.  See the GNU coding standards for more about this.

I see.

I've worked mostly with FreeBSD sources, where follow the reverse
principle.  The commit logs keep the WHY of each change there.

I'll have to reread the GNU coding standards.

Thanks for the explanation :)

Re: Minor src/xmenu.c change for older GCC versions

[Chong Yidong]

Richard Stallman <rms@gnu.org> writes:

>     > +  FRAME_PTR f;
>     > +
>     >    BLOCK_INPUT;
>     > -  FRAME_PTR f = check_x_frame (frame);
>     > -
>     > +  f = check_x_frame (frame);
>
> Please add a comment explaining why this is necessary, to make sure
> people don't take this out for elegance's sake.

Done.