Re: [PATCH] Split of normal mode (version 2)

[Bean]

On Sat, Apr 4, 2009 at 4:12 AM, Yoshinori K. Okuji <address@hidden> wrote:
> On Saturday 04 April 2009 04:49:36 Bean wrote:
>> On Sat, Apr 4, 2009 at 2:40 AM, Yoshinori K. Okuji <address@hidden> wrote:
>> > On Tuesday 31 March 2009 02:41:14 Bean wrote:
>> >> Hi,
>> >>
>> >> This new patch make some changes based on the discussion of previous
>> >> patch.
>> >>
>> >> 1, Move script engine to script/sh (sh.mod)
>> >> 2, Move generic menu code to menu (menu.mod)
>> >> 3, Move text menu viewer to menu/text (textmenu.mod)
>> >> 4, Move misc function to lib (misc.mod)
>> >> 5, Move setjmp to lib (setjmp.mod)
>> >
>> > I don't agree on the last two. Also, I don't like that you have just
>> > removed the rescue command.
>>
>> Hi,
>>
>> The reason for a separate misc.mod is that it contains pure function
>> and has no side effect, so other modules can use them freely. In the
>> other hand, normal.mod does a lot of tasks in the init function, it's
>> not a good place for common function. Besides, there are already
>> helper function in the lib directory, like crc and hexdump, it's not
>> piratical to put each one in a new module, we might just merge them
>> into one helper module.
>
> We can just put them in the normal.mod. What is wrong? Frankly, your argument
> reminds me of the old discussion about monolithic vs. micro kernels...
>

One of the problem for normal.mod dependency is its side effect. For
example, currently ls.mod depend on normal.mod just for
grub_normal_print_device_info. If we want to embed ls.mod in core.img,
we must embedded normal.mod as well, along with a lot of
initialization actions that we may not want.

>> The problem with setjmp is that it's platform dependent, so if we keep
>> it in normal.mod, we must define it in very platform rmk file. But now
>> setjmp is not used in normal.mod anymore, we could move it out, and
>> thus make normal.mod platform independent.
>
> setjmp is required for the switch between rescue mode and normal mode. Don't
> remove it. "Hey, this is easier to maintain" is not a good reason to drop an
> useful feature. If your intention is to reduce the maintenance cost, you can
> simply define a variable for common files in common.rmk and use it in each
> machine-specific rmk file.
>

No, setjmp is not required for the switch between rescue mode and
normal mode anymore. In fact, to enter rescue mode, you use:

handler reader rescue

To switch back to normal mode:

handler reader normal

It is not recursive, so no need for setjmp.

>> >> Now normal.mod only contains the reader code. To configure script
>> >> engine and viewer, you should add these lines at the beginning of
>> >> grub.cfg:
>> >>
>> >> insmod sh
>> >> handler parser sh
>> >> insmod textmenu
>> >
>> > I prefer a more sophisticated approach (note: I hate manual loading).
>> >
>> > For example, we can allow a config file to have a shebang, like "#!sh".
>> > If not specified, GRUB can assume that "sh" is used, and load it
>> > automatically. This kind of technique could even allow for using
>> > different languages in one setup.
>> >
>> > For textmenu, I think it makes sense to have a command "textmenu". Just
>> > like "boot", GRUB can execute "textmenu" implicitly if a config file
>> > defines any menu entry but does not execute any menu command. This way,
>> > textmenu is automatically loaded.
>>
>> I'm thinking about using environment variable to set handler, for
>> example, we could set it like this:
>>
>> set parser=sh
>> set menu=textmenu
>> set terminal_output=gfxterm
>>
>> We could use variable hooks to load the modules on demand.
>>
>> For textmenu, we could add some test in normal.mod. After the main
>> config file is parsed, if it still doesn't contain a menu viewer, we
>> load textmenu automatically.
>
> I agree that terminal_output (and so terminal_input as well) can be
> implemented as environment variables, but it is very questionable whether a
> parser or a menu interface can be considered as an environment variable. An
> important question is how you would determine the name of a module from the
> name of a parser or a menu interface.

We can generate a handler.lst by scanning source for
grub_parser_register, etc, just like commands.lst:

parser.sh: sh
reader.normal: normal
menu.textmenu: textmenu
terminal_outout.gfxterm: gfxterm

We can then read it in normal.mod and set the hooks for all handler classes.

-- 
Bean