new __strtol warning

  On Thu, 13 Nov 2014, Roland McGrath wrote:

> Perhaps we should start adding -Werror=foo for certain warnings, and
> -Werror=strict-prototypes is a fairly obvious safe candidate.

Using plain -Werror seems better than enabling it for particular warnings 
only - we just need an appropriate policy on selective disabling of 
warnings.

> 2. Change the definition to be a prototype.  This is a transformation we
>    have been generically doing, so it doesn't hurt.  I'm not sure what our
>    current scheme is for ensuring there is a prior declaration for every
>    non-static function when the definition uses a prototype so that
>    -Wstrict-prototypes doesn't catch it.  Nothing we have more automated
>    than human eyeballs will catch useless extra symbol names this way.

I've committed this patch that fixes those warnings by using a prototype 
definition for __strtol.

Tested for x86_64 that stripped installed shared libraries are
unchanged by this patch.

> 3. Rework your change so that it does not use the name __strtol at all,
>    while still using __strtoull et al.  This is the ideal solution for this
>    case, because strtol is a C89 function and so __strtol will never be
>    used anywhere.  It's always nice to clean the symbol table of useless
>    symbols and have the DWARF data use the canonical name for a function.

That seemed an excessive complication (as the extra name is harmless and 
simplifies the code).

2014-11-13  Joseph Myers  <joseph@codesourcery.com>

	* stdlib/strtol.c (__strtol): Use prototype definition.
  

> Using plain -Werror seems better than enabling it for particular warnings 
> only - we just need an appropriate policy on selective disabling of 
> warnings.

I was thinking that we could start with -Werror=foo for enabling things
piecemeal before we figure out everything we'd need to do for blanket
-Werror not to break.  And if we support a --disable-werror for older
compilers or whatnot (as vapier wanted), then -Werror=foo for specific safe
ones like strict-prototypes might make sense in that case too (though it's
probably fine for --disable-werror to disable all -Werrorness).

> I've committed this patch that fixes those warnings by using a prototype 
> definition for __strtol.

Thanks.

> > 3. Rework your change so that it does not use the name __strtol at all,
> >    while still using __strtoull et al.  This is the ideal solution for this
> >    case, because strtol is a C89 function and so __strtol will never be
> >    used anywhere.  It's always nice to clean the symbol table of useless
> >    symbols and have the DWARF data use the canonical name for a function.
> 
> That seemed an excessive complication (as the extra name is harmless and 
> simplifies the code).

Fair enough.  Extra names might be considered almost entirely harmless
(inflating .symtab alone is arguably no real harm).  But changing the
primary name of a function is some small harm, because it changes the DWARF
info and thus what name people see in a debugger.