Some comments on your work (note that I'm far from being a Hurd coding standards expert): *I don't think we use the /* * Implemented types */ comment style, even as "titles" for part of files. Just put /* Implemented types. */. Don't forget the dot at the end of your sentences and the two spaces in your comments. You can also separate different parts of a file by putting ^L characters into it (C-q C-l) with Emacs. This enable to use the forward-page/backward-page commands in Emacs. (Little advertisement :) : I enhanced a page-break-mode for emacs which display a nice line instead of the ^L, on http://www.emacswiki.org/cgi-bin/emacs-fr.pl/PageBreaks) *You put extra parenthesis, like in (l4_generic_bootinfo_t) (l4_boot_info());. These are not needed. *Try to avoid lines longer than 80 columns. Less than 78 columns is even better. *We prefer not to align the = vertically. *Try to avoid too much typecasts. For instance, you declare l4_word_t br = (l4_word_t) l4_generic_bootinfo_first_entry (bi) and then you either systematically typecast br to a (l4_generic_boot_rec_t) or to a (l4_generic_bootinfo_module_t). It would be much simpler to write: l4_generic_boot_rec_t br = l4_generic_bootinfo_first_entry (bi); l4_generic_bootinfo_module_t bootinfo_module = (l4_generic_bootinfo_module_t) br; or something like this (I don't know if br is even needed; the offset_next and types fields are common to both structures). *Don't declare your functions in source file: extern l4_word_t mbi_to_generic_bootinfo (multiboot_info_t*); because that make softwares harder to maintain. Put them in a header file (or create one). *The bootinfo.h files look fine, but could you implement the L4 compat interface too? I think it's just a matter of query-replace-regexp :) *Don't use "generic" libl4 names like _L4_BOOTINFO_MODULE, but define for them a GNU and a L4 compat name. *Please provide a Changelog entry along with your patches (but not as part of your patches). It's easier then to find out what you did, and to find bugs later. Don't be scared by all the observations I made here! Many of them are just matter of coding taste. Everything you wrote seems semantically fine, even if I did not read everything carefully yet. It will be easier to understand once you've supplied the Changelog entries :) I don't know if you should redo your patch, because I don't know if we want it to be integrated (nothing to do with you, just I don't know if we need it). Well I guess the bootinfo part can still be integrated in libl4. Marcus will be able to tell more on this, I hope. If so, I think there is plenty of other things to do that we will be happy to give you. Note that we require that you assign your changes to the FSF. So please send an email to [EMAIL PROTECTED] so that we can incorporate them. Hoping to talk to you soon, Thanks, Matthieu Date: Mon, 21 Mar 2005 22:38:58 +0000 In-Reply-To: <[EMAIL PROTECTED]> (Alexandre Buisse's message of "Mon, 28 Feb 2005 23:02:24 +0100") Message-ID: <[EMAIL PROTECTED]> User-Agent: Gnus/5.11 (Gnus v5.11) Emacs/22.0.50 (gnu/linux)
_______________________________________________ L4-hurd mailing list [email protected] http://lists.gnu.org/mailman/listinfo/l4-hurd
