C99

C99

[Tom Tromey]

I'd like to draw attention to this patch:

    http://sourceware.org/ml/gdb-patches/2013-06/msg00808.html

This points out that gdb has been unconditionally using a GCC extension,
apparently since at least 2010; the patch introducing the varargs define
in tracepoint.c was 7c56ce7 (2010-04-09).

The patch proposes replacing this with the corresponding C99 construct.

So, I'd like to propose we allow the use of C99 in gdb.  In particular I
think we ought to require a C99 preprocessor -- enabling this particular
patch to go in and also allowing the use of "//" comments.

Tom

Re: C99

[Mark Kettenis]

> From: Tom Tromey <tromey@redhat.com>
> Date: Tue, 16 Jul 2013 14:51:36 -0600
> 
> I'd like to draw attention to this patch:
> 
>     http://sourceware.org/ml/gdb-patches/2013-06/msg00808.html
> 
> This points out that gdb has been unconditionally using a GCC extension,
> apparently since at least 2010; the patch introducing the varargs define
> in tracepoint.c was 7c56ce7 (2010-04-09).
> 
> The patch proposes replacing this with the corresponding C99 construct.
> 
> So, I'd like to propose we allow the use of C99 in gdb.  In particular I
> think we ought to require a C99 preprocessor -- enabling this particular
> patch to go in and also allowing the use of "//" comments.

Perhaps it is time to move on and start requiring a C99 compiler for GDB.
But "//" comments are offensive to real C programmers! ;)

Seriously though.  This points out that such a switch has some
consequences for our coding standards.  We have a fairly consistent
coding style in GDB, which makes it easy for people to move around in
the codebase without getting distracted by the "looks" of the code.  I
think it's worth some effort to keep it that way.  And allowing "//"
comments isn't going to help.  I'd vote for not using them at all.

However, a more important C99 "misfeature" that affects the coding
standard is the possibility to declare varaibles anywhere in the code.
We should not allow this, except for declaring loop variables in a
for() statement.

Re: C99

[Doug Evans]

On Tue, Jul 16, 2013 at 2:22 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> From: Tom Tromey <tromey@redhat.com>
>> Date: Tue, 16 Jul 2013 14:51:36 -0600
>>
>> I'd like to draw attention to this patch:
>>
>>     http://sourceware.org/ml/gdb-patches/2013-06/msg00808.html
>>
>> This points out that gdb has been unconditionally using a GCC extension,
>> apparently since at least 2010; the patch introducing the varargs define
>> in tracepoint.c was 7c56ce7 (2010-04-09).
>>
>> The patch proposes replacing this with the corresponding C99 construct.
>>
>> So, I'd like to propose we allow the use of C99 in gdb.  In particular I
>> think we ought to require a C99 preprocessor -- enabling this particular
>> patch to go in and also allowing the use of "//" comments.
>
> Perhaps it is time to move on and start requiring a C99 compiler for GDB.
> But "//" comments are offensive to real C programmers! ;)
>
> Seriously though.  This points out that such a switch has some
> consequences for our coding standards.  We have a fairly consistent
> coding style in GDB, which makes it easy for people to move around in
> the codebase without getting distracted by the "looks" of the code.  I
> think it's worth some effort to keep it that way.  And allowing "//"
> comments isn't going to help.  I'd vote for not using them at all.

I don't see // as a serious issue, but I can live with it either way.

> However, a more important C99 "misfeature" that affects the coding
> standard is the possibility to declare varaibles anywhere in the code.
> We should not allow this, except for declaring loop variables in a
> for() statement.

Can you elaborate?

Re: C99

[Mark Kettenis]

> Date: Tue, 16 Jul 2013 14:40:24 -0700
> From: Doug Evans <dje@google.com>
> 
> On Tue, Jul 16, 2013 at 2:22 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
> >> From: Tom Tromey <tromey@redhat.com>
> >> Date: Tue, 16 Jul 2013 14:51:36 -0600
> >>
> >> I'd like to draw attention to this patch:
> >>
> >>     http://sourceware.org/ml/gdb-patches/2013-06/msg00808.html
> >>
> >> This points out that gdb has been unconditionally using a GCC extension,
> >> apparently since at least 2010; the patch introducing the varargs define
> >> in tracepoint.c was 7c56ce7 (2010-04-09).
> >>
> >> The patch proposes replacing this with the corresponding C99 construct.
> >>
> >> So, I'd like to propose we allow the use of C99 in gdb.  In particular I
> >> think we ought to require a C99 preprocessor -- enabling this particular
> >> patch to go in and also allowing the use of "//" comments.
> >
> > Perhaps it is time to move on and start requiring a C99 compiler for GDB.
> > But "//" comments are offensive to real C programmers! ;)
> >
> > Seriously though.  This points out that such a switch has some
> > consequences for our coding standards.  We have a fairly consistent
> > coding style in GDB, which makes it easy for people to move around in
> > the codebase without getting distracted by the "looks" of the code.  I
> > think it's worth some effort to keep it that way.  And allowing "//"
> > comments isn't going to help.  I'd vote for not using them at all.
> 
> I don't see // as a serious issue, but I can live with it either way.
> 
> > However, a more important C99 "misfeature" that affects the coding
> > standard is the possibility to declare varaibles anywhere in the code.
> > We should not allow this, except for declaring loop variables in a
> > for() statement.
> 
> Can you elaborate?

Code like this:

int
foobar(char *foo, int bar)
{
	sprintf(foo, "%d", bar)
	int j = strlen(foo);
	return j;
}

is bad if you're trained to look for variable declerations at the
start of a block.  C90 doesn't allow this; C99 changed that.  Most
hardcore C programmers consider this a bad decision by the standard
committe.  Most people do accept the following though:

int
foobar(int bar)
{
    int sum = 0;

    for (int i = 0; i < bar; i++)
        sum += i;


    return sum;
}

Re: C99

[Doug Evans]

On Wed, Jul 17, 2013 at 1:48 PM, Mark Kettenis <mark.kettenis@xs4all.nl> wrote:
>> > However, a more important C99 "misfeature" that affects the coding
>> > standard is the possibility to declare varaibles anywhere in the code.
>> > We should not allow this, except for declaring loop variables in a
>> > for() statement.
>>
>> Can you elaborate?
>
> Code like this:
>
> int
> foobar(char *foo, int bar)
> {
>         sprintf(foo, "%d", bar)
>         int j = strlen(foo);
>         return j;
> }
>
> is bad if you're trained to look for variable declerations at the
> start of a block.  C90 doesn't allow this; C99 changed that.  Most
> hardcore C programmers consider this a bad decision by the standard
> committe.  Most people do accept the following though:
>
> int
> foobar(int bar)
> {
>     int sum = 0;
>
>     for (int i = 0; i < bar; i++)
>         sum += i;
>
>
>     return sum;
> }

Yeah, except I'm not seeing why it's a problem.  Why is it a bad
decision?  What class of bugs does it cause?
I can see that it makes it easier to find all the locals, but I'm more
for declaring/initializing them close to their use.

Re: C99? No, portability.

[John Gilmore]

> > So, I'd like to propose we allow the use of C99 in gdb.  In particular I
> > think we ought to require a C99 preprocessor -- enabling this particular
> > patch to go in and also allowing the use of "//" comments.
> 
> Perhaps it is time to move on and start requiring a C99 compiler for GDB.

Mark said it correctly.  This change would "require" a C99 compiler.
Not just "allow the use of C99 in GDB".

I recommend that you NOT break compatability with older compilers for
gratuitous reasons.  For example, I still run systems based on Red Hat
7.3, which use gcc-2.96.  I can still compile modern GDB's on that
system.  (With the few portability patches below :-).)

If you switched to requiring C99 in new GDB releases, I could still
probably compile a modern GDB -- I'd just have to edit a hundred more
places where you've made use of some new construct where an old one
would do just as well.  And of course you wouldn't take my portability
patches back.  If you'd allow old-system GDB users to fix a C99 GDB so
that it didn't require C99 any more, what would be the point of
switching to requiring C99?

The particular patch that Tom pointed out would remove the use of
GCC-specific vararg CPP macros from gdbserver.  Fixing that would be a
good thing for portability.  I do suggest fixing it -- not by
requiring C99, but by fixing all the calls to those couple of macros
so that they don't require vararg macros, since you can't do vararg
macros portably.  Then when you were done, GDB would be even more
portable, rather than less portable.

	John

./dwarf2.c: In function `_bfd_dwarf2_cleanup_debug_info':
./dwarf2.c:3604: parse error before `struct'
./dwarf2.c:3609: `each' undeclared (first use in this function)

diff -ru gdb-7.5/bfd/dwarf2.c gdb-7.5-for-rh7.3/bfd/dwarf2.c
--- gdb-7.5/bfd/dwarf2.c	Wed May 16 06:57:11 2012
+++ gdb-7.5-for-rh7.3/bfd/dwarf2.c	Wed Jul 17 00:35:42 2013
@@ -3600,7 +3600,7 @@
 void
 _bfd_dwarf2_cleanup_debug_info (bfd *abfd, void **pinfo)
 {
-  struct dwarf2_debug *stash = (struct dwarf2_debug *) *pinfo;;
+  struct dwarf2_debug *stash = (struct dwarf2_debug *) *pinfo;
   struct comp_unit *each;
 
   if (abfd == NULL || stash == NULL)

elflink.c: In function `elf_link_add_object_symbols':
elflink.c:4730: warning: `idx' might be used uninitialized in this function

diff -ru gdb-7.5/bfd/elflink.c gdb-7.5-for-rh7.3/bfd/elflink.c
--- gdb-7.5/bfd/elflink.c	Mon Jul  9 01:17:34 2012
+++ gdb-7.5-for-rh7.3/bfd/elflink.c	Wed Jul 17 00:38:46 2013
@@ -4729,6 +4729,7 @@
 	  bfd_vma vlook;
 	  size_t i, j, idx;
 
+	  idx = 0;
 	  hlook = weaks;
 	  weaks = hlook->u.weakdef;
 	  hlook->u.weakdef = NULL;


gdbreplay.c: In function `expect':
gdbreplay.c:372: warning: `fromgdb' might be used uninitialized in this function

diff -ru gdb-7.5/gdb/gdbserver/gdbreplay.c gdb-7.5-for-rh7.3/gdb/gdbserver/gdbreplay.c
--- gdb-7.5/gdb/gdbserver/gdbreplay.c	Wed Jan  4 00:17:23 2012
+++ gdb-7.5-for-rh7.3/gdb/gdbserver/gdbreplay.c	Wed Jul 17 00:56:59 2013
@@ -369,7 +369,7 @@
 expect (FILE *fp)
 {
   int fromlog;
-  int fromgdb;
+  int fromgdb = 0;
 
   if ((fromlog = logchar (fp)) != ' ')
     {

cc1: warnings being treated as errors
tracepoint.c:848: warning: `used' attribute directive ignored

diff -ru gdb-7.5/gdb/gdbserver/tracepoint.c gdb-7.5-for-rh7.3/gdb/gdbserver/tracepoint.c
--- gdb-7.5/gdb/gdbserver/tracepoint.c	Sat Apr 28 23:28:30 2012
+++ gdb-7.5-for-rh7.3/gdb/gdbserver/tracepoint.c	Wed Jul 17 00:59:07 2013
@@ -845,7 +845,7 @@
 /* The linked list of all tracepoints.  Marked explicitly as used as
    the in-process library doesn't use it for the fast tracepoints
    support.  */
-IP_AGENT_EXPORT struct tracepoint *tracepoints ATTR_USED;
+IP_AGENT_EXPORT struct tracepoint *tracepoints /* ATTR_USED makes -Werror fail */;
 
 #ifndef IN_PROCESS_AGENT
 

xml-support.c: In function `gdb_xml_fetch_external_entity':
xml-support.c:513: storage size of `status' isn't known
xml-support.c: In function `gdb_xml_use_dtd':
xml-support.c:562: warning: implicit declaration of function `XML_UseForeignDTD'
xml-support.c:562: `XML_TRUE' undeclared (first use in this function)
xml-support.c:562: (Each undeclared identifier is reported only once
xml-support.c:562: for each function it appears in.)
xml-support.c: In function `gdb_xml_parse':
xml-support.c:579: storage size of `status' isn't known

diff -ru gdb-7.5/gdb/xml-support.c gdb-7.5-for-rh7.3/gdb/xml-support.c
--- gdb-7.5/gdb/xml-support.c	Wed Jan  4 00:27:58 2012
+++ gdb-7.5-for-rh7.3/gdb/xml-support.c	Wed Jul 17 00:54:44 2013
@@ -510,7 +510,7 @@
   struct gdb_xml_parser *parser = XML_GetUserData (expat_parser);
   XML_Parser entity_parser;
   const char *text;
-  enum XML_Status status;
+  int status;
 
   if (systemId == NULL)
     {
@@ -558,12 +558,14 @@
   XML_SetExternalEntityRefHandler (parser->expat_parser,
 				   gdb_xml_fetch_external_entity);
 
+#if 0  /* I'm not sure what to do here.  No decl for this function.  */
   /* Even if no DTD is provided, use the built-in DTD anyway.  */
   err = XML_UseForeignDTD (parser->expat_parser, XML_TRUE);
   if (err != XML_ERROR_NONE)
     internal_error (__FILE__, __LINE__,
 		    _("XML_UseForeignDTD failed: %s"),
 		    XML_ErrorString (err));
+#endif
 }
 
 /* Invoke PARSER on BUFFER.  BUFFER is the data to parse, which
@@ -576,7 +578,7 @@
 int
 gdb_xml_parse (struct gdb_xml_parser *parser, const char *buffer)
 {
-  enum XML_Status status;
+  int status;
   const char *error_string;
 
   gdb_xml_debug (parser, _("Starting:\n%s"), buffer);

Re: C99? No, portability.

[Doug Evans]

On Wed, Jul 17, 2013 at 1:11 AM, John Gilmore <gnu@toad.com> wrote:
>> > So, I'd like to propose we allow the use of C99 in gdb.  In particular I
>> > think we ought to require a C99 preprocessor -- enabling this particular
>> > patch to go in and also allowing the use of "//" comments.
>>
>> Perhaps it is time to move on and start requiring a C99 compiler for GDB.
>
> Mark said it correctly.  This change would "require" a C99 compiler.
> Not just "allow the use of C99 in GDB".
>
> I recommend that you NOT break compatability with older compilers for
> gratuitous reasons.  For example, I still run systems based on Red Hat
> 7.3, which use gcc-2.96.  I can still compile modern GDB's on that
> system.  (With the few portability patches below :-).)

gdb successfully moved from K&R to C89, so it's not like we haven't
been through this before.

C99 is 14 years old.  How many people still require C89 vs how many
have long since moved on?

RE: C99? No, portability.

[John Kearney]

Well c99 may be 14 years old but it still isn't fully supported.
http://en.wikipedia.org/wiki/C99 



-----Original Message-----
From: gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] On Behalf Of Doug Evans
Sent: Donnerstag, 18. Juli 2013 00:38
To: John Gilmore
Cc: Mark Kettenis; Tom Tromey; gdb
Subject: Re: C99? No, portability.

On Wed, Jul 17, 2013 at 1:11 AM, John Gilmore <gnu@toad.com> wrote:
>> > So, I'd like to propose we allow the use of C99 in gdb.  In 
>> > particular I think we ought to require a C99 preprocessor -- 
>> > enabling this particular patch to go in and also allowing the use of "//" comments.
>>
>> Perhaps it is time to move on and start requiring a C99 compiler for GDB.
>
> Mark said it correctly.  This change would "require" a C99 compiler.
> Not just "allow the use of C99 in GDB".
>
> I recommend that you NOT break compatability with older compilers for 
> gratuitous reasons.  For example, I still run systems based on Red Hat 
> 7.3, which use gcc-2.96.  I can still compile modern GDB's on that 
> system.  (With the few portability patches below :-).)

gdb successfully moved from K&R to C89, so it's not like we haven't been through this before.

C99 is 14 years old.  How many people still require C89 vs how many have long since moved on?

Re: C99? No, portability.

[Doug Evans]

The things that are unsupported seem pretty esoteric as far as gdb is concerned.
And at any rate, I'm sure we can find a useful subset.
I can imagine we did the same thing when we transitioned to C89
(especially with respect to library support).
I can also imagine we're still avoiding things added in C89 (for
portability reasons), but I'm glad we transitioned.

On Thu, Jul 18, 2013 at 8:26 AM, John Kearney <jokearney@qnx.com> wrote:
> Well c99 may be 14 years old but it still isn't fully supported.
> http://en.wikipedia.org/wiki/C99
>
>
>
> -----Original Message-----
> From: gdb-owner@sourceware.org [mailto:gdb-owner@sourceware.org] On Behalf Of Doug Evans
> Sent: Donnerstag, 18. Juli 2013 00:38
> To: John Gilmore
> Cc: Mark Kettenis; Tom Tromey; gdb
> Subject: Re: C99? No, portability.
>
> On Wed, Jul 17, 2013 at 1:11 AM, John Gilmore <gnu@toad.com> wrote:
>>> > So, I'd like to propose we allow the use of C99 in gdb.  In
>>> > particular I think we ought to require a C99 preprocessor --
>>> > enabling this particular patch to go in and also allowing the use of "//" comments.
>>>
>>> Perhaps it is time to move on and start requiring a C99 compiler for GDB.
>>
>> Mark said it correctly.  This change would "require" a C99 compiler.
>> Not just "allow the use of C99 in GDB".
>>
>> I recommend that you NOT break compatability with older compilers for
>> gratuitous reasons.  For example, I still run systems based on Red Hat
>> 7.3, which use gcc-2.96.  I can still compile modern GDB's on that
>> system.  (With the few portability patches below :-).)
>
> gdb successfully moved from K&R to C89, so it's not like we haven't been through this before.
>
> C99 is 14 years old.  How many people still require C89 vs how many have long since moved on?

Re: C99

[Yao Qi]

On 07/17/2013 05:22 AM, Mark Kettenis wrote:
> Perhaps it is time to move on and start requiring a C99 compiler for GDB.
> But "//" comments are offensive to real C programmers!;)
>
> Seriously though.  This points out that such a switch has some
> consequences for our coding standards.  We have a fairly consistent
> coding style in GDB, which makes it easy for people to move around in
> the codebase without getting distracted by the "looks" of the code.  I
> think it's worth some effort to keep it that way.  And allowing "//"
> comments isn't going to help.  I'd vote for not using them at all.
>

+1.  Probably we can move to C99 now.  Personally, I prefer "/* */" to 
"//" in the comments.

> However, a more important C99 "misfeature" that affects the coding
> standard is the possibility to declare varaibles anywhere in the code.
> We should not allow this, except for declaring loop variables in a
> for() statement.

I agree.

-- 
Yao (齐尧)

Re: C99

[Eli Zaretskii]

> From: Tom Tromey <tromey@redhat.com>
> Date: Tue, 16 Jul 2013 14:51:36 -0600
> 
> So, I'd like to propose we allow the use of C99 in gdb.  In particular I
> think we ought to require a C99 preprocessor -- enabling this particular
> patch to go in and also allowing the use of "//" comments.

I can understand the C99 requirement (although many GNU projects still
don't) where it allows better programming practices.  But the //
comment style doesn't make anything easier or more efficient, it's
just different.

Re: C99

[Doug Evans]

On Tue, Jul 16, 2013 at 8:48 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Tom Tromey <tromey@redhat.com>
>> Date: Tue, 16 Jul 2013 14:51:36 -0600
>>
>> So, I'd like to propose we allow the use of C99 in gdb.  In particular I
>> think we ought to require a C99 preprocessor -- enabling this particular
>> patch to go in and also allowing the use of "//" comments.
>
> I can understand the C99 requirement (although many GNU projects still
> don't) where it allows better programming practices.  But the //
> comment style doesn't make anything easier or more efficient, it's
> just different.

Well, yes and no.

It can be what some contributors are used to.  Saying // is not
allowed is another quirky imposition one has to remember.  To me, it
doesn't get in the way of reading code the way other differences can
(indentation, SymbolSpellingStyle, etc.).  Note that we're already
using // in code:  While it's in /* ... */ comments in, e.g. queue.h
and gnulib's string.h, for obvious simplicity reasons the code in the
comments isn't less readable.  Plus if we import code from elsewhere
we don't want to be in the business of changing // to /**/.

Re: C99

[Yao Qi]

On 07/18/2013 01:54 AM, Doug Evans wrote:
> Note that we're already
> using // in code:  While it's in /* ... */ comments in, e.g. queue.h

because we can't use /* */ inside the comments.

-- 
Yao (齐尧)

Re: C99

[Doug Evans]

On Wed, Jul 17, 2013 at 7:46 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 07/18/2013 01:54 AM, Doug Evans wrote:
>>
>> Note that we're already
>> using // in code:  While it's in /* ... */ comments in, e.g. queue.h
>
>
> because we can't use /* */ inside the comments.

That's the "for obvious simplicity reason" I was referring to. :-)