all messages for Emacs-related lists mirrored at yhetil.org
 help / color / mirror / code / Atom feed
* Minor src/xmenu.c change for older GCC versions
@ 2006-11-03 21:50 Giorgos Keramidas
  2006-11-04  3:21 ` Chong Yidong
  0 siblings, 1 reply; 7+ messages in thread
From: Giorgos Keramidas @ 2006-11-03 21:50 UTC (permalink / raw)


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

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-03 21:50 Minor src/xmenu.c change for older GCC versions Giorgos Keramidas
@ 2006-11-04  3:21 ` Chong Yidong
  2006-11-05  7:07   ` Richard Stallman
  0 siblings, 1 reply; 7+ messages in thread
From: Chong Yidong @ 2006-11-04  3:21 UTC (permalink / raw)
  Cc: emacs-devel

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.

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-04  3:21 ` Chong Yidong
@ 2006-11-05  7:07   ` Richard Stallman
  2006-11-05  8:00     ` Giorgos Keramidas
  2006-11-05 17:31     ` Chong Yidong
  0 siblings, 2 replies; 7+ messages in thread
From: Richard Stallman @ 2006-11-05  7:07 UTC (permalink / raw)
  Cc: keramida, emacs-devel

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

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-05  7:07   ` Richard Stallman
@ 2006-11-05  8:00     ` Giorgos Keramidas
  2006-11-05 12:06       ` Eli Zaretskii
  2006-11-05 17:31     ` Chong Yidong
  1 sibling, 1 reply; 7+ messages in thread
From: Giorgos Keramidas @ 2006-11-05  8:00 UTC (permalink / raw)
  Cc: Chong Yidong, emacs-devel

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

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-05  8:00     ` Giorgos Keramidas
@ 2006-11-05 12:06       ` Eli Zaretskii
  2006-11-05 13:18         ` Giorgos Keramidas
  0 siblings, 1 reply; 7+ messages in thread
From: Eli Zaretskii @ 2006-11-05 12:06 UTC (permalink / raw)
  Cc: emacs-devel

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

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-05 12:06       ` Eli Zaretskii
@ 2006-11-05 13:18         ` Giorgos Keramidas
  0 siblings, 0 replies; 7+ messages in thread
From: Giorgos Keramidas @ 2006-11-05 13:18 UTC (permalink / raw)
  Cc: emacs-devel

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

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

* Re: Minor src/xmenu.c change for older GCC versions
  2006-11-05  7:07   ` Richard Stallman
  2006-11-05  8:00     ` Giorgos Keramidas
@ 2006-11-05 17:31     ` Chong Yidong
  1 sibling, 0 replies; 7+ messages in thread
From: Chong Yidong @ 2006-11-05 17:31 UTC (permalink / raw)
  Cc: keramida, emacs-devel

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.

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

end of thread, other threads:[~2006-11-05 17:31 UTC | newest]

Thread overview: 7+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2006-11-03 21:50 Minor src/xmenu.c change for older GCC versions Giorgos Keramidas
2006-11-04  3:21 ` Chong Yidong
2006-11-05  7:07   ` Richard Stallman
2006-11-05  8:00     ` Giorgos Keramidas
2006-11-05 12:06       ` Eli Zaretskii
2006-11-05 13:18         ` Giorgos Keramidas
2006-11-05 17:31     ` Chong Yidong

Code repositories for project(s) associated with this external index

	https://git.savannah.gnu.org/cgit/emacs.git
	https://git.savannah.gnu.org/cgit/emacs/org-mode.git

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.