Re: Patch 2/3 for topic/libposix

[Eric Blake]

On 11/11/2010 06:09 PM, Bruce Korb wrote:
> This patch modifies lib/error.c so that should someone
> call it without having set program_name, then a message
> is printed and abort() called instead of seg faulting.

> Subject: [PATCH 2/3] give a sensible error message when error() is called 
> without a set program name

It's nice to provide a ChangeLog entry, either as a patch to ChangeLog
itself or as the commit message (or both; if you use Jim's vc-dwim pacage).

> 
> ---
>  lib/error.c |   50 +++++++++++++++++++++++++++++---------------------
>  1 files changed, 29 insertions(+), 21 deletions(-)
> 
> diff --git a/lib/error.c b/lib/error.c
> index ed9dba0..19065d8 100644
> --- a/lib/error.c
> +++ b/lib/error.c
> @@ -106,7 +106,10 @@ char *strerror_r ();
>  
>  /* The calling program should define program_name and set it to the
>     name of the executing program.  */
> -extern char *program_name;
> +#ifdef HPUX
> +extern
> +#endif
> +char *program_name;

I'm not convinced this change is correct.

>  
> +static inline void
> +pr_prog_name (void)
> +
> +  if (error_print_progname)
> +    (*error_print_progname) ();
> +  else if (program_name == NULL)
> +    {
> +#if _LIBC
> +      __fxprintf (NULL, "program_name not set\n");
> +#else
> +      fprintf (stderr, "program_name not set\n");
> +#endif

mid-method #ifdefs are harder to maintain.  Easier is an up-front:

#ifndef _LIBC
# define __fxprintf fprintf
#endif

so that the rest of the code can just blindly follow glibc's conventions.

> @@ -296,16 +322,7 @@ error (int status, int errnum, const char *message, ...)
>  #ifdef _LIBC
>    _IO_flockfile (stderr);
>  #endif
> -  if (error_print_progname)
> -    (*error_print_progname) ();
> -  else
> -    {
> -#if _LIBC
> -      __fxprintf (NULL, "%s: ", program_name);
> -#else
> -      fprintf (stderr, "%s: ", program_name);
> -#endif
> -    }
> +  pr_prog_name ();

More importantly, error.c is a file that we sync with glibc.  We should
probably work on updating the synchronization between the two places, as
well as cleaning up existing style, so that it would be easier to push
this patch back into glibc sources.  I'm a bit reluctant to push this
big of a patch into gnulib without first gauging glibc response; if we
could reduce the size of the changes and still resolve the root cause
that you are trying to address, that might be better.

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature