This is the mail archive of the gdb-patches@sourceware.org mailing list for the GDB project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]
Other format: [Raw text]

Re: [PATCH] Fix Gold/strip discrepancies for PR 11786


On 11/05/2013 05:04 PM, Doug Evans wrote:
> On Mon, Nov 4, 2013 at 6:34 PM, Tom Tromey <tromey@redhat.com> wrote:
>>>> (void)
>>
>> Doug> We don't have any such style rules for testcases.
>> Doug> But ok, done.
>>
>> Doug> Going forward though, for my own patch reviews of other people's code,
>> Doug> what's the story here?
>>
>> In this case, "()" is not idiomatic C, whereas "(void)" is.
>> This is not the same as a coding style rule.
> 
> I'm not sure how to read this.
> 
> Is this an argument for requiring (void) in all testsuite C function
> definitions?
> It's ok by me, but it seems to me it's not a requirement today as
> there are plenty of existing examples, including recent ones.  OTOH,
> if there is such a requirement we'd better get it written down so we
> can refer to it when requesting corrections in patches, and so people
> can know ahead of time what's expected.  Obviously the rules state
> this for gdb itself, but it's been my understanding that these rules
> explicitly do not apply to the testsuite, and this understanding has
> been affirmed from time to time.  All I'm asking for is clarity and
> consistency.
> 
> Or is this just a point about a bad use of the word "style"?
> By itself "style" is a pretty nondescript word.
> Alas I'm not one for always assigning names with precision.
> If this isn't a style issue, coding *or* otherwise, let me know what to call it.
> 
> If this is something else, let me know that too. :-)

I wouldn't say this is a style issue, or even idiomatic
vs not idiomatic.  I'd just call it poorly written C code.  In C,
unlike C++, () is different from (void).  (void) declares a
function that takes no parameters while () declares a function
that take any number of parameters.  I.e., this compiles fine:

 void foo () {}
 void bar () { foo (1, 2, 3); }

While this does not:

 void foo (void) {}
 void bar (void) { foo (1, 2, 3); }

So, in C, use of () is usually a bug waiting to happen.

In terms of coding style, GDB should be able to debug
code of all sort of styles, so we aren't usually strict
on having tests follow GDB's own coding style.  Myself,
I'll only point out coding style issues in test code if
most of the code is clearly following GDB's coding style,
except a place or too.  E.g., if there's a whole series
of comments that have double space after period except
this one spot.  Or if the test has 10 functions and function
calls, and all them have space before parens, except one
place, etc.

Personally, I'm all for "write for what mean", so if
the test's functions in question are not even remotely about
() vs (void), I also see no point in writing (), and I'd
probably also point it out if I were to review it, expecting
it to be just the usual C vs C++ confusion.

-- 
Pedro Alves


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]