Re: [2338] 2009-06-17 Felix Zielcke <fzielcke@z-51.de>

Re: [2338] 2009-06-17 Felix Zielcke <fzielcke@z-51.de>

[Robert Millan]

Hi Felix,

This commit looks wrong.  grub_file_open is already supposed to set
grub_errno and grub_errmsg appropiately.

Is there a specific problem you wanted to fix here?

On Wed, Jun 17, 2009 at 05:19:23PM +0000, Felix Zielcke wrote:
> Revision: 2338
>           http://svn.sv.gnu.org/viewvc/?view=rev&root=grub&revision=2338
> Author:   fzielcke
> Date:     2009-06-17 17:19:23 +0000 (Wed, 17 Jun 2009)
> Log Message:
> -----------
> 2009-06-17  Felix Zielcke  <fzielcke@z-51.de>
> 
> 	* loader/i386/linux.c (grub_cmd_linux): Set grub_error if the
> 	file can't be found.
> 	* loader/i386/pc/linux.c (grub_cmd_linux): Likewise.
> 
> Modified Paths:
> --------------
>     trunk/grub2/ChangeLog
>     trunk/grub2/loader/i386/linux.c
>     trunk/grub2/loader/i386/pc/linux.c
> 
> Modified: trunk/grub2/ChangeLog
> ===================================================================
> --- trunk/grub2/ChangeLog	2009-06-17 13:47:37 UTC (rev 2337)
> +++ trunk/grub2/ChangeLog	2009-06-17 17:19:23 UTC (rev 2338)
> @@ -1,3 +1,9 @@
> +2009-06-17  Felix Zielcke  <fzielcke@z-51.de>
> +
> +	* loader/i386/linux.c (grub_cmd_linux): Set grub_error if the
> +	file can't be found.
> +	* loader/i386/pc/linux.c (grub_cmd_linux): Likewise.
> +
>  2009-06-17  Vladimir Serbinenko  <phcoder@gmail.com>
>  
>  	Fix newline handling
> 
> Modified: trunk/grub2/loader/i386/linux.c
> ===================================================================
> --- trunk/grub2/loader/i386/linux.c	2009-06-17 13:47:37 UTC (rev 2337)
> +++ trunk/grub2/loader/i386/linux.c	2009-06-17 17:19:23 UTC (rev 2338)
> @@ -600,7 +600,10 @@
>  
>    file = grub_file_open (argv[0]);
>    if (! file)
> -    goto fail;
> +    {
> +      grub_error (GRUB_ERR_FILE_NOT_FOUND, "file not found");
> +      goto fail;
> +    }
>  
>    if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))
>      {
> 
> Modified: trunk/grub2/loader/i386/pc/linux.c
> ===================================================================
> --- trunk/grub2/loader/i386/pc/linux.c	2009-06-17 13:47:37 UTC (rev 2337)
> +++ trunk/grub2/loader/i386/pc/linux.c	2009-06-17 17:19:23 UTC (rev 2338)
> @@ -69,7 +69,10 @@
>  
>    file = grub_file_open (argv[0]);
>    if (! file)
> -    goto fail;
> +    {
> +      grub_error (GRUB_ERR_FILE_NOT_FOUND, "file not found");
> +      goto fail;
> +    }
>  
>    if ((grub_size_t) grub_file_size (file) > grub_os_area_size)
>      {
> 
> 
> 
> 

-- 
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."

Re: [2338] 2009-06-17 Felix Zielcke <fzielcke@z-51.de>

[Robert Millan]

On Sat, Jun 20, 2009 at 03:43:51PM +0200, Robert Millan wrote:
> 
> Hi Felix,
> 
> This commit looks wrong.  grub_file_open is already supposed to set
> grub_errno and grub_errmsg appropiately.
> 
> Is there a specific problem you wanted to fix here?

Hi,

I reverted this.  If there's something that needs fixing here, let us know
about it! :-)

Thanks

-- 
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."

Re: [2338] 2009-06-17 Felix Zielcke <fzielcke@z-51.de>

[Felix Zielcke]

Am Samstag, den 20.06.2009, 15:43 +0200 schrieb Robert Millan:
> Hi Felix,
> 
> This commit looks wrong.  grub_file_open is already supposed to set
> grub_errno and grub_errmsg appropiately.
> 
> Is there a specific problem you wanted to fix here?

If the loaded file can't be found, the menuentry code just continuesa
and initrd then complains that you must load the kernel first.

> On Wed, Jun 17, 2009 at 05:19:23PM +0000, Felix Zielcke wrote:
> > Revision: 2338
> >           http://svn.sv.gnu.org/viewvc/?view=rev&root=grub&revision=2338
> > Author:   fzielcke
> > Date:     2009-06-17 17:19:23 +0000 (Wed, 17 Jun 2009)
> > Log Message:
> > -----------
> > 2009-06-17  Felix Zielcke  <fzielcke@z-51.de>
> > 
> > 	* loader/i386/linux.c (grub_cmd_linux): Set grub_error if the
> > 	file can't be found.
> > 	* loader/i386/pc/linux.c (grub_cmd_linux): Likewise.
> > 
> > Modified Paths:
> > --------------
> >     trunk/grub2/ChangeLog
> >     trunk/grub2/loader/i386/linux.c
> >     trunk/grub2/loader/i386/pc/linux.c
> > 
> > Modified: trunk/grub2/ChangeLog
> > ===================================================================
> > --- trunk/grub2/ChangeLog	2009-06-17 13:47:37 UTC (rev 2337)
> > +++ trunk/grub2/ChangeLog	2009-06-17 17:19:23 UTC (rev 2338)
> > @@ -1,3 +1,9 @@
> > +2009-06-17  Felix Zielcke  <fzielcke@z-51.de>
> > +
> > +	* loader/i386/linux.c (grub_cmd_linux): Set grub_error if the
> > +	file can't be found.
> > +	* loader/i386/pc/linux.c (grub_cmd_linux): Likewise.
> > +
> >  2009-06-17  Vladimir Serbinenko  <phcoder@gmail.com>
> >  
> >  	Fix newline handling
> > 
> > Modified: trunk/grub2/loader/i386/linux.c
> > ===================================================================
> > --- trunk/grub2/loader/i386/linux.c	2009-06-17 13:47:37 UTC (rev 2337)
> > +++ trunk/grub2/loader/i386/linux.c	2009-06-17 17:19:23 UTC (rev 2338)
> > @@ -600,7 +600,10 @@
> >  
> >    file = grub_file_open (argv[0]);
> >    if (! file)
> > -    goto fail;
> > +    {
> > +      grub_error (GRUB_ERR_FILE_NOT_FOUND, "file not found");
> > +      goto fail;
> > +    }
> >  
> >    if (grub_file_read (file, &lh, sizeof (lh)) != sizeof (lh))
> >      {
> > 
> > Modified: trunk/grub2/loader/i386/pc/linux.c
> > ===================================================================
> > --- trunk/grub2/loader/i386/pc/linux.c	2009-06-17 13:47:37 UTC (rev 2337)
> > +++ trunk/grub2/loader/i386/pc/linux.c	2009-06-17 17:19:23 UTC (rev 2338)
> > @@ -69,7 +69,10 @@
> >  
> >    file = grub_file_open (argv[0]);
> >    if (! file)
> > -    goto fail;
> > +    {
> > +      grub_error (GRUB_ERR_FILE_NOT_FOUND, "file not found");
> > +      goto fail;
> > +    }
> >  
> >    if ((grub_size_t) grub_file_size (file) > grub_os_area_size)
> >      {
> > 
> > 
> > 
> > 
> 
-- 
Felix Zielcke

Re: [2338] 2009-06-17 Felix Zielcke <fzielcke@z-51.de>

[Robert Millan]

On Sun, Jun 28, 2009 at 12:23:36PM +0200, Felix Zielcke wrote:
> Am Samstag, den 20.06.2009, 15:43 +0200 schrieb Robert Millan:
> > Hi Felix,
> > 
> > This commit looks wrong.  grub_file_open is already supposed to set
> > grub_errno and grub_errmsg appropiately.
> > 
> > Is there a specific problem you wanted to fix here?
> 
> If the loaded file can't be found, the menuentry code just continuesa
> and initrd then complains that you must load the kernel first.

Thanks.  Looking at the patch, the only possible explanation for this is that
grub_file_open() returned 0 but didn't set grub_errno.

I'll try to find a moment to debug this.

-- 
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."