Re: Remove parameter names from libiberty.h

Re: Remove parameter names from libiberty.h

[Marc Espie]

You wrote:

>Personally, I like parameter names as documentation, and I'd recommend
>moving the inclusion of libiberty.h higher in opcodes/m88k-dis.c so
>that "mode" isn't clobbered by the time we get these prototypes, but
>it's not worth arguing over IMO.

I'm siding with Alan there, but for the sake of it, I believe you
deserve an explanation for why it's the right thing to do, and not
just a matter of taste and style.

Simply-put: parameter names in prototypes will get you, sooner or
later.

C is an annoying language without enough namespaces, and where the
preprocessor can tramp all over the place.

From time to time, it breaks function declarations. I've had my share
of 3rd party software that includes a bunch of system includes 
like sys/types.h, and then defines a swap32 function that just happens
to collide with the macro of the same name in my system includes.

This doesn't happen with standard function declarations, because, well,
they're standard, so people will tend to avoid that namespace, even for
preprocessing purposes... well, not always, and when gcc started warning
for log(), I first had to check my standard that, yes, indeed log is always
reserved, even if you don't include math.h, and that all the system 
administration utilities that just wanted to output information to a journal
were wrong to name that function log.

Enter parameter names. Same trouble, all over again, but worse. See, no
standard defines the names that parameters should have. There's no way
to enforce anything there, since C is strictly positional, and doesn't
care one bit if you write extern void f(int foo);  in one file and then
write the actual function
void
f(int bar)
{
}

Okay, enough background, that you definitely know already. So, parameter
names are not reserved, and no API tells you to avoid them, and so, sooner
or later, someone with a loaded preprocessor uses just the name you
carefully crafted for your prototype.  Basically, it's a collision between
two system-wide problems: header files that get included all over the place,
and a preprocessor that doesn't understand the language beyond lexing.

You can HAVE parameter names, but sooner or later, this will break
something somewhere, and generate A LOT of head scratching, especially
from beginners who haven't yet figured gcc -save-temps or gcc -dM.

And it translates to further headaches down the road when you have
parameter names, and you work on a BIG project, and so you have to work
around preprocessor collisions, and so you reorder your header files, up
to a point, when you finally figure out that it can't work, because you've
got header inter-dependencies going one way, and parameter names + macros
going the other way. So you start adding MORE include guards, as if the
forest of #ifdef wasn't bad enough already...

You will end up getting it to work, after having lost hours to it (generally,
header file breakage occurs just after you went to bed, having launched
a full compile that's supposed to take four hours)... only for it to be so
brittle that it breaks the next time someone changes anything.

Okay, for sure, there's a lot of preprocessor/header issues that have
nothing to do with parameter names. But parameter names is a heavy 
maintenance issue, doesn't lead to robust source code, and frankly, it
doesn't give you all that much in the area of documentation.

Put simply, it's not worth it, and real bad software engineering practice.
Put even more simply, there's a parameter name with your name on it,
and sooner or later, it will get you.

So, in OpenBSD, we killed them. We don't go out of our way to fix every
userland program includes, because `if it ain't broke, don't fix it', but
when we write new stuff, we don't put it in.

If you really want comments, well, add comments.
Some people like this style:
extern FILE *fopen_unlocked (const char * /* path */, const char * /* mode */);
personally I prefer:
/* file = fopen_unlocked(path, mode); */
extern FILE *fopen_unlocked (const char *, const char *);


Quick plug for a good book: Lakos `Large scale C++'. Really worth it.

Re: Remove parameter names from libiberty.h

[Thorsten Glaser]

Marc Espie dixit:

>C is an annoying language without enough namespaces, and where the
>preprocessor can tramp all over the place.

Such as
| Apr 16  OpenBSD/1 is ELF, 2003
in calendar(1), right?

(i386 got predefined by cpp)

//mirabile

Remove parameter names from libiberty.h

[Paul Schlie]

> Kaveh R. Ghazi writes:
> As noted here:
> http://sourceware.org/ml/binutils/2005-04/msg00269.html
> 
> The new f*open_unlocked function prototypes in libiberty.h are in
> conflict with binutils sources because one of the parameter names
> ("mode") gets defined to "31" and the build dies.

Why not alternatively fix the real problem (as you had noted), as opposed to
indirectly establishing the policy that libiberty prototypes don't include
parameter names (as it doesn't seem like the true solution to the problem).

Re: Remove parameter names from libiberty.h

[Andrew Pinski]

> 
> > Kaveh R. Ghazi writes:
> > As noted here:
> > http://sourceware.org/ml/binutils/2005-04/msg00269.html
> > 
> > The new f*open_unlocked function prototypes in libiberty.h are in
> > conflict with binutils sources because one of the parameter names
> > ("mode") gets defined to "31" and the build dies.
> 
> Why not alternatively fix the real problem (as you had noted), as opposed to
> indirectly establishing the policy that libiberty prototypes don't include
> parameter names (as it doesn't seem like the true solution to the problem).

I don't know why I am replying to you but ...
Anyways there is no other way to fix the problem correctly.  If we change
the parameter name, someone else might have defined it so we get into
a cycle.

-- Pinski

Re: Remove parameter names from libiberty.h

[H. J. Lu]

On Thu, Apr 14, 2005 at 12:17:11PM -0400, Andrew Pinski wrote:
> > 
> > > Kaveh R. Ghazi writes:
> > > As noted here:
> > > http://sourceware.org/ml/binutils/2005-04/msg00269.html
> > > 
> > > The new f*open_unlocked function prototypes in libiberty.h are in
> > > conflict with binutils sources because one of the parameter names
> > > ("mode") gets defined to "31" and the build dies.
> > 
> > Why not alternatively fix the real problem (as you had noted), as opposed to
> > indirectly establishing the policy that libiberty prototypes don't include
> > parameter names (as it doesn't seem like the true solution to the problem).
> 
> I don't know why I am replying to you but ...
> Anyways there is no other way to fix the problem correctly.  If we change
> the parameter name, someone else might have defined it so we get into
> a cycle.
> 

There are many prototypes in libiberty.h with names.


H.J.

RE: Remove parameter names from libiberty.h

[Dave Korn]

----Original Message----
>From: H. J. Lu
>Sent: 14 April 2005 18:18

> On Thu, Apr 14, 2005 at 12:17:11PM -0400, Andrew Pinski wrote:
>>> 
>>>> Kaveh R. Ghazi writes:
>>>> As noted here:
>>>> http://sourceware.org/ml/binutils/2005-04/msg00269.html
>>>> 
>>>> The new f*open_unlocked function prototypes in libiberty.h are in
>>>> conflict with binutils sources because one of the parameter names
>>>> ("mode") gets defined to "31" and the build dies.
>>> 
>>> Why not alternatively fix the real problem (as you had noted), as
>>> opposed to indirectly establishing the policy that libiberty prototypes
>>> don't include parameter names (as it doesn't seem like the true
>>> solution to the problem). 
>> 
>> I don't know why I am replying to you but ...
>> Anyways there is no other way to fix the problem correctly.  If we change
>> the parameter name, someone else might have defined it so we get into
>> a cycle.
>> 
> 
> There are many prototypes in libiberty.h with names.
> 


  My two cents is that it's asking for trouble to #define an all-lower case
word, particularly an extremely common one such as 'mode'.  I had a quick
grep but couldn't figure out where it's coming from.  I guess that sort of
thing is vaguely-ok-if-innately-fragile if it's being defined by the
m88k-dis source file, but if there's a public header in either binutils or
libiberty that's doing it, I'd think it ought to change.



    cheers,
      DaveK
-- 
Can't think of a witty .sigline today....

Re: Remove parameter names from libiberty.h

[Ian Lance Taylor]

"Dave Korn" <dave.korn@artimi.com> writes:

>   My two cents is that it's asking for trouble to #define an all-lower case
> word, particularly an extremely common one such as 'mode'.  I had a quick
> grep but couldn't figure out where it's coming from.  I guess that sort of
> thing is vaguely-ok-if-innately-fragile if it's being defined by the
> m88k-dis source file, but if there's a public header in either binutils or
> libiberty that's doing it, I'd think it ought to change.

It was coming from include/opcode/m88k.h, and it's already been fixed
there by Paul Brook.

Still, I don't think we should use parameter names for well understood
functions.  There is no ideal policy here, but omitting unnecessary
parameter names seems to me to be wise regardless.

Ian

libiberty.h

[Alan Modra]

This change

2005-04-11  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* libiberty.h (fopen_unlocked, fdopen_unlocked, freopen_unlocked):
	Provide prototypes for new functions.

causes

gcc -DHAVE_CONFIG_H -I. -I/src/binutils-current/opcodes -I. -D_GNU_SOURCE -I. -I/src/binutils-current/opcodes -I../bfd -I/src/binutils-current/opcodes/../include -I/src/binutils-current/opcodes/../bfd -I/src/binutils-current/opcodes/../intl -I../intl -W -Wall -Wstrict-prototypes -Wmissing-prototypes -Werror -g -O -c /src/binutils-current/opcodes/m88k-dis.c -o m88k-dis.o
In file included from /src/binutils-current/opcodes/m88k-dis.c:27:
/src/binutils-current/opcodes/../include/libiberty.h:55: error: parse error before numeric constant
/src/binutils-current/opcodes/../include/libiberty.h:55: warning: function declaration isn't a prototype
/src/binutils-current/opcodes/../include/libiberty.h:56: error: parse error before numeric constant
/src/binutils-current/opcodes/../include/libiberty.h:56: warning: function declaration isn't a prototype
/src/binutils-current/opcodes/../include/libiberty.h:57: error: parse error before numeric constant
/src/binutils-current/opcodes/../include/libiberty.h:57: warning: function declaration isn't a prototype

Preprocessing shows

extern FILE *fopen_unlocked (const char *path, const char *31);
extern FILE *fdopen_unlocked (int fildes, const char *31);
extern FILE *freopen_unlocked (const char *path, const char *31, FILE *stream);

Please remove the needless parameter names.

-- 
Alan Modra
IBM OzLabs - Linux Technology Centre

Remove parameter names from libiberty.h

[Kaveh R. Ghazi]

As noted here:
http://sourceware.org/ml/binutils/2005-04/msg00269.html

The new f*open_unlocked function prototypes in libiberty.h are in
conflict with binutils sources because one of the parameter names
("mode") gets defined to "31" and the build dies.

Personally, I like parameter names as documentation, and I'd recommend
moving the inclusion of libiberty.h higher in opcodes/m88k-dis.c so
that "mode" isn't clobbered by the time we get these prototypes, but
it's not worth arguing over IMO.

So here's a patch to do as Alan asked.  Let me know if I should
install it.

		--Kaveh



2005-04-13  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>

	* libiberty.h (fopen_unlocked, fdopen_unlocked, freopen_unlocked):
	Remove parameter names.

diff -rup orig/egcc-CVS20050413/include/libiberty.h egcc-CVS20050413/include/libiberty.h
--- orig/egcc-CVS20050413/include/libiberty.h	2005-04-13 19:33:34.502555400 -0400
+++ egcc-CVS20050413/include/libiberty.h	2005-04-13 19:34:12.465784112 -0400
@@ -52,9 +52,9 @@ extern "C" {
    the stream is setup to avoid any multi-threaded locking.  Otherwise
    return the FILE pointer unchanged.  */
 
-extern FILE *fopen_unlocked (const char *path, const char *mode);
-extern FILE *fdopen_unlocked (int fildes, const char *mode);
-extern FILE *freopen_unlocked (const char *path, const char *mode, FILE *stream);
+extern FILE *fopen_unlocked (const char *, const char *);
+extern FILE *fdopen_unlocked (int, const char *);
+extern FILE *freopen_unlocked (const char *, const char *, FILE *);
 
 /* Build an argument vector from a string.  Allocates memory using
    malloc.  Use freeargv to free the vector.  */

Re: Remove parameter names from libiberty.h

[Ian Lance Taylor]

"Kaveh R. Ghazi" <ghazi@caip.rutgers.edu> writes:

> 2005-04-13  Kaveh R. Ghazi  <ghazi@caip.rutgers.edu>
> 
> 	* libiberty.h (fopen_unlocked, fdopen_unlocked, freopen_unlocked):
> 	Remove parameter names.

Yes, please.  Thanks.

While I agree with parameter name documentation in general, I don't
think we need it for these functions, since the underlying functions
are well known.

Ian

Re: Remove parameter names from libiberty.h

[Thorsten Glaser]

Kaveh R. Ghazi dixit:

>Personally, I like parameter names as documentation, and I'd recommend

What do the GNU Coding Styles say in this regard?
If nothing, maybe we should discuss that point
a bit more lengthy and then add it?

The BSD coding styles say to NOT use parameter names
in declarations, only in the actual function, FWIW.
(It took me some time to get used to it, too.)

bye,
//mirabile

Re: Remove parameter names from libiberty.h

[DJ Delorie]

Opinions follow...

Since libiberty has a separate texinfo documentation that's supposed
to document all the parameters, and since most of libiberty's
functions are common system functions anyway, IMHO it's not as big a
deal if libiberty.h doesn't have names of parameters in it.

In DJGPP we got around this by adding underscores, but that was
acceptable only because it was the C library and that namespace was
reserved for it.

Commented out parameter names gets messy fast, esp for functions with
lots of parameters.