grub-devel
[Top][All Lists]
Advanced

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

Re: [PATCH] File access library for lua


From: Pavel Roskin
Subject: Re: [PATCH] File access library for lua
Date: Tue, 23 Jun 2009 18:10:36 -0400

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.

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?

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

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.

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.

-- 
Regards,
Pavel Roskin




reply via email to

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