Yay!! Does this change the minimum GCC version needed to build? Thanks!! -S
On Jan 1, 2013, at 6:42 AM, Colin Watson <cjwat...@ubuntu.com> wrote: > (Part zero was a patch I already committed that dealt with some trivial > cases.) > > As I mentioned on #grub, and following up on > https://lists.gnu.org/archive/html/grub-devel/2009-04/msg00406.html and > thread, I'm working on a patch set to eliminate nested functions from > GRUB. I'd like to add a couple of extra reasons to those given by Pavel > nearly four years ago: > > * The trampolines used when taking the address of a nested function are > a net cost in terms of code size. With my full set of patches so > far, the i386-pc (compressed) kernel gets 52 bytes smaller and a > sample core image profile (biosdisk ext2 part_msdos lvm mdraid1x) > gets 39 bytes smaller. That's admittedly not lots, but there are > some situations on i386-pc that are very close to the wire and where > every byte in the core image counts. > > * Even several years on, we have failed to solve all the build system > problems associated with nested functions. See Savannah bugs #36669 > and #37938. > > * It is revealing that if you search the web for "gcc nested functions" > or similar, you find several GCC bug reports from GRUB developers. > We appear to be in a small minority of free programs making use of > this facility, and I for one would rather concentrate on producing a > high-quality boot loader rather than fighting arcane compiler > features. > > The vast majority of the nested functions we use - or at any rate the > particularly problematic ones that involve trampolines - are iterator > hook functions. In the general case, un-nesting these involves passing > a context structure as an additional argument to the hook function. I > have adopted a mostly formulaic approach to this, exemplified by the > attached patch which converts grub_pci_iterate to the new scheme. My > approach, which I'll spell out for the sake of those maintaining > patches, is as follows: > > * Whenever a function (usually *_iterate) takes a hook function pointer > "foo" as a parameter, add an additional "void *foo_data" parameter > immediately after it. > > * If there is not already a typedef for the hook function type, add > one. > > * Update all implementations of the hook type in question to take an > additional "void *data" parameter. > > * Move each hook that was previously a nested function to the top level > of its file, and mark it static. > > * If a hook requires no local variables from its parent function, mark > the data parameter __attribute__ ((unused)) and pass NULL when > calling it (either directly or via the relevant *_iterate function). > > * If a hook only requires a single local variable from its parent > function, pass a reference to that variable as its data parameter, > and have the hook declare an pointer variable at the top initialised > to data (e.g. "static int *found = data"). > > * If a hook requires more than one local variable from its parent > function, declare "struct <name-of-parent>_ctx" with the necessary > variables, and convert both the hook and the parent to access the > variables in question via that structure. I made use of GCC's > non-constant aggregate automatic initialisers extension when > initialising the context structure in the parent, which I found > clearest. > > * If a hook has a reasonably clear name already, leave it alone. > However, if it is called something excessively general such as > "hook", then rename it either to something describing its purpose or > else simply to "<name-of-parent>_iter". > > * Remove any NESTED_FUNC_ATTR from hook declarations, and from any > pre-existing typedef for the hook function type. > > I have a number of patches mostly ready to go, but I'd prefer to make > sure that this general approach is agreed before preparing and sending > more than one of them. I'd like to work one *_iterate function at a > time (except where multiple iterators are entangled in a stack such that > we need to change several at once, as is the case in parts of the > disk/filesystem stacks), as that's roughly the minimum sensible unit and > it makes it reasonably easy to grep for missing changes. > > Please review. > > Thanks, > > -- > Colin Watson [cjwat...@ubuntu.com] > <unnest-pci-iterate.patch> > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel