Re: [PATCH] Warning if grub.cfg not found

[Robert Millan <rmh@aybabtu.com>]

Hi

On Sat, Aug 23, 2008 at 04:43:14PM +0200, Carles Pina i Estany wrote:
> Index: normal/cmdline.c
> ===================================================================
> --- normal/cmdline.c	(revision 1826)
> +++ normal/cmdline.c	(working copy)
> @@ -137,12 +137,17 @@ grub_cmdline_run (int nested)
>  {
>    grub_normal_init_page ();
>    grub_setcursor (1);
> +
> +  if ( nested == -1 )

nested was intended to be a "boolean";  this changes its meaning, so the name
becomes confusing.  I think there's no need to reuse the variable in this part
of GRUB, and it'd be fine to add a new one IMO.  However ...

> +     grub_printf ("\n\
> + WARNING: GNU GRUB couldn't open /boot/grub/grub.cfg\n\
> + Falling back to GNU GRUB Command Line\n\n");

... this looks like something that belongs whereever the decision to fall
back is taken.  Then once the problem is handled there, you don't need to
tell the lower layer whether to print a message or not.

Also, I think there are two separate cases:

  - grub.cfg is there but can't be opened (we need to tell the user about
    _why_ via grub_print_error()).

  - grub.cfg is simply not there (perhaps the user intended that).

and the messages should be somewhat different for each one.

-- 
Robert Millan

  The DRM opt-in fallacy: "Your data belongs to us. We will decide when (and
  how) you may access your data; but nobody's threatening your freedom: we
  still allow you to remove your data and not access it at all."