On Fri, Jul 18, 2008 at 10:46 PM, Colin D Bennett <[EMAIL PROTECTED]> wrote: > On Thu, 17 Jul 2008 11:24:23 +0800 > Bean <[EMAIL PROTECTED]> wrote: > >> On Mon, Jul 7, 2008 at 8:29 AM, Bean <[EMAIL PROTECTED]> wrote: >> > Hi, >> > >> > First of all, we can still keep rescue and normal command. But >> > instead of depending on normal.mod, normal command depends on >> > module arg, which is an option parser. Also, these two type of >> > commands are of the same command set. In fact, module arg is >> > implemented as a pre parser, which goes through the list of >> > arguments and extract the options. In the case of rescue command, >> > the pre parser field is null, which means it wants to parse options >> > itself. >> > >> > Then, I think of a new structure to represent all configurable >> > handlers of grub. Different types of handler have different fields, >> > but they all share a command header: >> > >> > struct grub_handler >> > { >> > .next, >> > .name, >> > .init, >> > .fini >> > }; >> > >> > Same type of handlers are linked together. We first define an enum >> > to list all types. For example: >> > >> > enum { >> > GRUB_HANDLER_INPUT, >> > GRUB_HANDLER_OUTPUT, >> > GRUB_HANDLER_CONSOLE, >> > GRUB_HANDLER_MENU, >> > GRUB_HANDLER_SCRIPT, >> > GRUB_HANDLER_NUM >> > }; >> > >> > Then, we define an array to point to the head of handler linked >> > list: grub_handler[GRUB_HANDLER_NUM]; >> > >> > Head is the default selection. When we insert a new handler module, >> > it would automatically become the new default, although we can >> > switch back to old handler using a command. >> > >> > Here are more details about different handlers: >> > >> > input: >> > This is the input component of terminal: >> > >> > struct grub_handler_input >> > { >> > .next, >> > .name, >> > .init, >> > .fini, >> > .checkkey, >> > .getkey >> > .flags, >> > }; >> > >> > output: >> > This is the output component of terminal: >> > >> > struct grub_handler_output >> > { >> > .next, >> > .name, >> > .init, >> > .fini, >> > .putchar, >> > .getcharwidth, >> > .getxy, >> > .gotoxy, >> > .cls, >> > .setcolorstate, >> > .setcursor, >> > .flags, >> > }; >> > >> > console interface: >> > It represent the grub console, users type commands and execute them >> > line by line. >> > >> > struct grub_handler_console >> > { >> > .next, >> > .name, >> > .init, >> > .fini, >> > .run >> > }; >> > >> > menu interface: >> > It represent the menu, users select a menu item and execute it. >> > >> > struct grub_handler_menu >> > { >> > .next, >> > .name, >> > .init, >> > .fini, >> > .run >> > }; >> > >> > script engine: >> > It's responsible for parsing config file to get the menu list, and >> > execution of commands. >> > >> > struct grub_handler_script >> > { >> > .next, >> > .name, >> > .init, >> > .fini, >> > .readconfig >> > .getmenu >> > .execute >> > }; >> > >> > The handlers are independent of each other. When they need >> > something, they called specific function of the default handler. >> > For example, to read a key from the console, we can use >> > grub_handler[GRUB_HANDLER_INPUT]->getkey. Also, to get the list of >> > items to be displayed on screen, the menu handler can call >> > grub_handler[GRUB_HANDLER_SCRIPT]->getmenu. >> >> Any comment for this idea ? > > I like the idea, especially the idea of separating the script engine > and menu loading code from the character-terminal-based menu system. > > I wonder whether it would be better just to have separate variables for > the different handler types like > > struct grub_handler_input grub_handler_input; > struct grub_handler_output grub_handler_output; > struct grub_handler_console grub_handler_console; > > instead of an array indexed by a constant representing the type, for > the following reasons: > > 1. You would have to use casting to be able to call the functions in > the handler other than the common 4 (next, name, init, fini). For > instance, you couldn't do > grub_handler[GRUB_HANDLER_INPUT]->getkey() > since the type of grub_handler is struct grub_handler[]. > > 2. If we did use casting for each call to a handler, it is more error > prone since we would not get compiler type checking and could > accidentally write GRUB_HANDLER_INPUT when we meant GRUB_HANDLER_OUTPUT > with obvious consequences. > > 3. It is more code to write to do > grub_handler[GRUB_HANDLER_INPUT]->getkey() > instead of > grub_handler_input->getkey() > > > Now, suppose we did define grub_handler_input, grub_handler_output, > etc. Then if there were a reason to iterate over all types of > handlers, or treat handlers generically for some other reason, we could > certainly also create and array of pointers to the handlers for that > purpose: > > extern struct grub_handler *grub_handlers[GRUB_HANDLER_NUM]; > extern const char *grub_handler_types[GRUB_HANDLER_NUM]; > > void print_current_handlers (void) > { > int i; > for (i = 0; i < GRUB_HANDLER_NUM; i++) > grub_printf ("handler for %s: %s\n", > grub_handler_types[i], > grub_handlers[i]); > } > > We would then have to make sure the list of all handlers was kept in > sync with the separate variables. This would mean a little more code > for each handler type (a global "set current handler" function for > each) but we would probably rarely add a new handler type so it would > not be a great burden. I don't see a need for this iteration over all > handlers, anyway.
Hi, I like the idea. In fact, I hate using cast, it makes the code ugly. Also, we can use generic code to handle them, define a structure like this: { { "input", &grub_handler_input_head}, { "output", &grub_handler_output_head}, {0, 0} }; As the first few fields of handler is the same, we can use casting to access them. -- Bean _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel