Re: [PATCH] File access library for lua

[Bean]

On Wed, Jun 24, 2009 at 6:10 AM, Pavel Roskin<address@hidden> wrote:
> On Tue, 2009-06-23 at 17:27 +0800, Bean wrote:
>> Hi,
>>
>> Some bug fix for osdetect.lua, it also detect windows 98/me, freedos,
>> msdos and freebsd.
>
> Why FressDOS and FressBSD?  I assume it's typos.  Why isn't Linux
> capitalized?  MS-DOS is written with a dash.  "Windows Vista bootmgr"
> should be "Windows Vista" and "Windows NT/2000/XP loader" should be
> "Windows NT/2000/XP".  It's not like we are just booting the loaders.
>

Oh, fixed typos now. As for "Windows Vista bootmgr"  and "Windows
NT/2000/XP loader", I just copy them from 10_windows.in, I guess there
is a reason for it there.

> inird should be initrd.  Please add check for the Fedora style names for
> initrd, namely "initrd-KVER.img".  Or maybe you just missed ".img" in
> the second check?
>

Add the test for initrd-KVER.img. Is there any distro that uses
initrd-KVER ? if not, I can just remove the second test.

>> Extend the function of grub.file_exist to allow testing multiple names
>> at the same time, this simplify osdetect.lua.
>
> The change to grub_lua_file_exist() is dubious.  It's not clear why the
> requirement is that all files exist.  Maybe I don't know the style of
> lua, but I think it's wrong to hardcode the AND logic just because one
> script would benefit from it.

As lua can detect the number of parameter quite easily, I just think
it'd be a waste not to utilize it. If you always call it with one
parameter, then it's equivalent to the previous file_exist. It's just
a little more code in c, but it can reduce the number of calls between
c and lua.

>
> If we consider e.g. the wildcard expansion in make, it will return a
> non-empty value if any file exists, i.e. the OR logic is used.
>

We can add a new function that uses OR logic instead of AND, for
example, grub.file_exist_any, and the current AND version can be
called grub.file_exist_all.

> I suggest that you split the lua.mod changes and osdetect.lua.  The
> later is obviously a bikeshed issue that can be discussed for a long
> time.  The former needs a more technical consideration.

Ok.

>
> --
> Regards,
> Pavel Roskin
>
>
> _______________________________________________
> Grub-devel mailing list
> address@hidden
> http://lists.gnu.org/mailman/listinfo/grub-devel
>

-- 
Bean

lua_file_3.diff
Description: Text Data

osdetect.gz
Description: GNU Zip compressed data