This is the mail archive of the binutils@sources.redhat.com mailing list for the binutils 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: src/binutils ChangeLog coffdump.c nlmconv.c nl ...


Daniel Jacobowitz <drow@mvista.com> writes:

> On Sun, Feb 10, 2002 at 11:13:05AM -0800, Ian Lance Taylor wrote:
> > drow@sources.redhat.com writes:
> > 
> > > 	* nlmconv.c: Add PARAMS ((const time_t *)) to localtime
> > > 	prototype.  Prototype main.
> > > 	* nlmheader.y: Add PARAMS ((int)) to strerror prototype.
> > 
> > Please don't make these changes.  The localtime change in particular
> > will cause the binutils to fail to compile on some platforms, those on
> > which localtime is declare in the system header files to take time_t *
> > rather than const time_t *.
> > 
> > Never use a full prototype when declaring functions provided the
> > system.  It can only cause conflicts and, except in the case of a
> > prototype which uses an ellipsis, can never help.  If you feel that
> > you must use a full prototype, you must add a configure test to
> > confirm that it will compile correctly.
> 
> I assume you're referring to localtime here; strerror was already
> handled this way in nlmconv.c, directly above where I added a prototype
> for localtime.

I think it's unwise to use a prototype for strerror(), but I admit
that it's unlikely to cause harm.  The prototype for localtime() can
clearly cause harm.

> In this case, should we turn off the warning I was fixing,
> -Wstrict-prototypes, or should I add configury for localtime's
> declaration?

Personally, I would say neither: keep the -Wstrict-prototypes warning,
which is often useful, but the ignore the warning in this particular
case.  Write a sed script to wipe out the warning so that you can spot
warnings of real problems instead.

Of course if you want to write a configure test for localtime's
declaration, that's OK with me.  But it doesn't help solve any real
problem.

localtime() is declared in nlmconv.c because it does not return int.
It's declared because some systems provide localtime() but do not
declare it in <time.h>.  Adding the prototype to the declaration
doesn't help anything except warning elimination, and it hurts the
ability to compile the file on some systems.

Ian


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