Re: [PATCH] Bug fix for ext2.c

[Bean]

On Sun, Mar 15, 2009 at 11:37 PM, Robert Millan <address@hidden> wrote:
> On Sun, Mar 15, 2009 at 11:33:18AM +0100, Felix Zielcke wrote:
>> Am Sonntag, den 15.03.2009, 14:00 +0800 schrieb Bean:
>>
>> > Hi,
>>
>> Hi Bean,
>>
>> > Oh right, this one should be ok.
>>
>> +    grub_error (GRUB_ERR_BAD_FS, "not an xfs filesystem");
>> This should probable be `not an ext2 filesystem' :)
>
> Why override grub_errmsg anyway?  "not ext2" isn't going to be more useful
> than a "read out of range" text when displayed to the user.

Hi,

I think the ext2 error message is better. The "read out of range"
error is such a general message, it tells us little about where it
goes wrong, while with "not ext2 filesystem",. we know that the error
is generated in grub_ext2_mount.

Although, a more informative way is to write it like this:

  if (grub_errno == GRUB_ERR_OUT_OF_RANGE)
    grub_error (GRUB_ERR_BAD_FS, "not an ext2 filesystem (out of range)");

But that seems a little overdo for me. Besides, fs drivers such as
xfs.c, hfsplus.c use simple message for out of range error.

-- 
Bean