"Neal H. Walfield" <[EMAIL PROTECTED]> writes: > I know know why you call this a handler; it seems to me that it is > just a semi-generic list package. Am I missing something?
Perhaps. I can better explain how this can be used and give a small example. GRUB has several types of handlers. I hope a handler is the right word, please correct me if it's not. Examples of handlers are filesystems, terminals, partitioning schemes, commands, etc. A handler usually consists of a struct with function pointers and a pointer to the next handler of its kind. Let's focus on filesystems. To implement the filesystem handler, we defined 3 basic functions. You need to be able to register and unregister filesystems. It should be possible to iterate over all filesystems. grub_file_open, for example, iterates over all filesystems to see if it detects a certain filesystem layout on some disk. What we currently did is rewriting grub_*_register, grub_*_unregister and grub_*_iterate. It's boring to rewrite such functions all the time and this results in duplicated code. So it is not a list in the classical sense, it does not contain data. > You can find a slighly more flexible and generic implementation here: > > > http://cvs.savannah.gnu.org/viewvc/hurd-l4/viengoos/list.h?root=hurd&view=markup > > I've been using that for a while and am quite satisfied with it. > > Perhaps you'll find it useful. It certainly looks good, thanks for the suggestion. However, I do not think we have the same goals. For example, I focus on size and do not need many features. > Two comments: > > At Fri, 29 Aug 2008 14:36:56 +0200, > Marco Gerards wrote: >> +void >> +grub_handler_unregister (grub_handler_t *head, grub_handler_t handler) >> +{ >> + grub_handler_t *p, q; >> + >> + for (p = head, q = *p; q; p = &(q->next), q = q->next) > ^^^^^^ ^^^^^^^^^^^ > > This is a bit inconsistent. > > for (p = head, q = *head; q; p = &(q->next), q = q->next) > > or > > for (p = head, q = *p; q; p = &(q->next), q = *p) > > > Or, more succinctly: > > for (p = head; (q = *p); p = &(q->next)) You are right, I simply blindly copied this from existing code. I can make this more consistent. >> +int >> +grub_handler_iterate (grub_handler_t head, >> + int (*hook) (const grub_handler_t handler)) >> +{ >> + grub_handler_t p; >> + >> + for (p = head; p; p = p->next) >> + if (hook (p)) >> + break; >> + >> + return 0; >> +} > > A suggestion: when HOOK returns a non-zero value, return that from the > function: > > for (p = head; p; p = p->next) > { > int ret = hook (p); > if (ret) > return ret; > } > return 0; Well spotted! Normally we use: if (hook (p)) return 1; This is what happens when you blindly copy existing code, it might not meet the behavior you expect :-/ -- Marco _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org http://lists.gnu.org/mailman/listinfo/grub-devel
