grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: grub-probe detects ext4 wronly as ext2


From: Robert Millan
Subject: Re: grub-probe detects ext4 wronly as ext2
Date: Sat, 30 Aug 2008 13:17:53 +0200
User-agent: Mutt/1.5.13 (2006-08-11)

On Mon, Aug 11, 2008 at 04:14:00PM +0200, Javier Martín wrote:
> >> > This overrides the grub_errno and grub_errmsg provided by grub_disk_read 
> >> > and
> >> > replaces them with values that hide the true problem.  If there was a 
> >> > disk
> >> > read error, we really want to know about it from the lower layer.
> >> Well, the old version did just the same (even worse, because the message
> >> was generic). What would be the correct path of action here? I mean, how
> >> can we propagate the error messages?
> >
> > It shouldn't call grub_error().
> 
> So in the cases the fail is caused by an underlying error (like I/O)
> the code should just return failure and leave the old error in errno
> and errmsg?

Yes.

>  static struct grub_ext2_data *
>  grub_ext2_mount (grub_disk_t disk)
>  {
>    struct grub_ext2_data *data;
> +  const char *local_error = 0;

Please use NULL for pointers.

> -  if (grub_errno)
> +  if (grub_errno != GRUB_ERR_NONE)

Why?

> -    goto fail;
> +    EXT2_DRIVER_MOUNT_FAIL(0);

I find very little use in this.  I assume it's supposed to simplify things,
but in fact it adds an extra level of indirection for someone who's reading
the code.  It provides no runtime improvement, and it's inconsistent with what
we do elsewhere.

> +  /* Only call grub_error if the fail was _not_ caused by underlying errors. 
>  */

No need to document this.  It's the same deal in a huge amount of routines
throurough the GRUB source.

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




reply via email to

[Prev in Thread] Current Thread [Next in Thread]