Re: [PATCH] kern/err.c + disk/raid.c error handling fixes

[Marco Gerards <mgerards@xs4all.nl>]

Hi Felix,

Felix Zielcke <fzielcke@z-51.de> writes:

[...]

> 2008-08-14  Felix Zielcke  <fzielcke@z-51.de>
>
>         * include/grub.err.h (grub_err_printf): New function prototype.

grub.err.h -> grub/err.h

>         * util/misc.c (grub_err_printf): New function.
>         * kern/misc.c [! GRUB_UTIL] (grub_err_printf): New alias for
>         grub_printf.
>         * kern/err.c (grub_print_error): Use grub_err_printf.

Looks fine to me.


This patch looks clean enough for me :-).  If you can correct the
following (and previous ;)) comments, it is ready to be committed.

> Index: kern/err.c
> ===================================================================
> --- kern/err.c	(Revision 1804)
> +++ kern/err.c	(Arbeitskopie)
> @@ -1,7 +1,7 @@
>  /* err.c - error handling routines */
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2005,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2005,2007,2008  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -121,14 +121,14 @@ grub_print_error (void)
>    do
>      {
>        if (grub_errno != GRUB_ERR_NONE)
> -        grub_printf ("error: %s\n", grub_errmsg);
> +        grub_err_printf ("error: %s\n", grub_errmsg);
>      } 
>    while (grub_error_pop ());
>    
>    /* If there was an assert while using error stack, report about it.  */
>    if (grub_error_stack_assert)
>      {
> -      grub_printf ("assert: error stack overflow detected!\n");
> +      grub_err_printf ("assert: error stack overflow detected!\n");
>        grub_error_stack_assert = 0;
>      }
>  }
> Index: kern/misc.c
> ===================================================================
> --- kern/misc.c	(Revision 1804)
> +++ kern/misc.c	(Arbeitskopie)
> @@ -133,6 +133,10 @@ grub_printf (const char *fmt, ...)
>  
>    return ret;
>  }  
> +#ifndef GRUB_UTIL
> +int grub_err_printf (const char *fmt, ...)
> +__attribute__ ((alias("grub_printf")));
> +#endif
>  
>  void
>  grub_real_dprintf (const char *file, const int line, const char *condition,
> Index: include/grub/err.h
> ===================================================================
> --- include/grub/err.h	(Revision 1804)
> +++ include/grub/err.h	(Arbeitskopie)
> @@ -1,7 +1,7 @@
>  /* err.h - error numbers and prototypes */
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2005,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2005,2007,2008 Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -64,5 +64,6 @@ void EXPORT_FUNC(grub_fatal) (const char
>  void EXPORT_FUNC(grub_error_push) (void);
>  int EXPORT_FUNC(grub_error_pop) (void);
>  void EXPORT_FUNC(grub_print_error) (void);
> +int EXPORT_FUNC(grub_err_printf) (const char *fmt, ...) __attribute__ ((format (printf, 1, 2)));

Can you break the line somehow?  It is too long.


>  #endif /* ! GRUB_ERR_HEADER */
> Index: util/misc.c
> ===================================================================
> --- util/misc.c	(Revision 1804)
> +++ util/misc.c	(Arbeitskopie)
> @@ -1,6 +1,6 @@
>  /*
>   *  GRUB  --  GRand Unified Bootloader
> - *  Copyright (C) 2002,2003,2005,2006,2007  Free Software Foundation, Inc.
> + *  Copyright (C) 2002,2003,2005,2006,2007,2008  Free Software Foundation, Inc.
>   *
>   *  GRUB is free software: you can redistribute it and/or modify
>   *  it under the terms of the GNU General Public License as published by
> @@ -74,6 +74,18 @@ grub_util_error (const char *fmt, ...)
>    exit (1);
>  }
>  
> +int
> +grub_err_printf (const char *fmt, ...)
> +{
> +  va_list ap;
> +  int ret;
> +  
> +  va_start (ap, fmt);
> +  ret = vfprintf (stderr, fmt, ap);
> +  va_end (ap);
> +
> +  return ret;
> +}
>  void *
>  xmalloc (size_t size)

Missing newline between the functions above.

--
Marco