Re: Portability fix for argp/argp.h.

[Ulrich Drepper <drepper at redhat dot com>]

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Simon Josefsson wrote:

> Index: argp-help.c
> ===================================================================
>  [...]
> +#ifndef __attribute
> +# define __attribute(xyz)     /* Ignore */
> +#endif

This is  wrong.  gcc does not define such a macro which means you
effectively disable to use of attributes.


> +#ifndef _LIBC
> +# if !HAVE_DECL_STRERROR
> +char *strerror ();
> +# endif
> +#endif

If the argp code can end up in multi-threaded code (and there is not
reason to assume this isn't possible) you'll have to add support for
strerror_r().  This applies the several places.


>  #include "argp.h"
>  #include "argp-fmtstream.h"
>  #include "argp-namefrob.h"
> @@ -1665,6 +1674,32 @@ void __argp_help (const struct argp *arg
>  weak_alias (__argp_help, argp_help)
>  #endif
>  
> +char *__argp_basename(char *name)
> +{
> +  char *short_name = strrchr(name, '/');

Just one of many places with missing space before the open parenthesis.
 Also frequently missing is correct indentation of preprocessor directives.

And the function should not be defined for libc, there is no reason for
that.  We have our own, possible better implementation.


> +__argp_short_program_name(const struct argp_state *state)
> +{
> +  if (state)
> +    return state->name;
> +#if HAVE_DECL_PROGRAM_INVOCATION_SHORT_NAME || defined _LIBC
> +  return program_invocation_short_name;
> +#elif HAVE_DECL_PROGRAM_INVOCATION_NAME
> +  return __argp_basename(program_invocation_name);
> +#else
> +  /* FIXME: What now? Miles suggests that it is better to use NULL,
> +     but currently the value is passed on directly to fputs_unlocked,
> +     so that requires more changes. */
> +#if __GNUC__
> +# warning No reasonable value to return
> +#endif /* __GNUC__ */
> +  return "";
> +#endif
> +}

What's the reason for this ugly function?  Why should a variable not be
used?


>       __THROW;
> +/* Used for extracting the program name from argv[0] */
> +extern char *_argp_basename(char *name) __THROW;
> +extern char *__argp_basename(char *name) __THROW;
> +
> +/* Getting the program name given an argp state */
> +extern char *_argp_short_program_name(const struct argp_state *state) __THROW;
> +extern char *__argp_short_program_name(const struct argp_state *state) __THROW;
>  

The declarations for these functions definitely need not be in an
installed header.  I.e., they don't belong in <argp.h>.

- -- 
- --------------.                        ,-.            444 Castro Street
Ulrich Drepper \    ,-----------------'   \ Mountain View, CA 94041 USA
Red Hat         `--' drepper at redhat.com `---------------------------
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.2.1 (GNU/Linux)

iD8DBQE/GxP72ijCOnn/RHQRApzKAKCN7vtE2M28JRBnb/Og/A8y873crACgmJaN
xVJnTYi9lFidEt6wF5Ivpn4=
=9kZ2
-----END PGP SIGNATURE-----