From mboxrd@z Thu Jan 1 00:00:00 1970 Received: from list by lists.gnu.org with archive (Exim 4.71) id 1Tt2j5-0003r2-T3 for mharc-grub-devel@gnu.org; Wed, 09 Jan 2013 15:58:19 -0500 Received: from eggs.gnu.org ([208.118.235.92]:53154) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt2j3-0003pd-DX for grub-devel@gnu.org; Wed, 09 Jan 2013 15:58:18 -0500 Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1Tt2ix-00050P-Nd for grub-devel@gnu.org; Wed, 09 Jan 2013 15:58:17 -0500 Received: from mail-wi0-f176.google.com ([209.85.212.176]:55308) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1Tt2ix-00050I-DU for grub-devel@gnu.org; Wed, 09 Jan 2013 15:58:11 -0500 Received: by mail-wi0-f176.google.com with SMTP id hm6so813898wib.15 for ; Wed, 09 Jan 2013 12:58:10 -0800 (PST) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20120113; h=x-received:message-id:date:from:user-agent:mime-version:to:subject :references:in-reply-to:x-enigmail-version:content-type; bh=xRwd4UJLJ68cbqg0LA/+uQX+2csnISM906jJwNoN4pE=; b=o9fSeCnMg4UkFnPh8DI55zg2vDrqcxCpxJxYS+cbpHU9RrrtNAd0Ozti6VfZLtc7U6 isnD75SSZGhrFFe/RUSXmBIO+eY2oBwn064TjjJ4hG/mp7RGNDQZzHSeqsrFO+c4DxJw aHxMNuOltZdEcaFOvZKfMQ3ky8zOR8RDI6+DzgFGFO8M1jSB5S0s2wWq5R59Qe+oMFm2 lZdEi7+zxk0pDJWPgNjPn5ajTd/AycGe1QitVFZzN5nxhSj4S34lYWFWg2PyBLM9DKjU IurpaVrSo0icHpz83O9Qv3vVzaRKDhP6/fpnk8e/WdFLHMyC8JzNAdKbxyCjcPJ8r8gD ZvJQ== X-Received: by 10.194.80.73 with SMTP id p9mr9869951wjx.4.1357765090468; Wed, 09 Jan 2013 12:58:10 -0800 (PST) Received: from debian.x201.phnet ([217.193.148.98]) by mx.google.com with ESMTPS id s10sm6031854wiw.4.2013.01.09.12.58.08 (version=TLSv1/SSLv3 cipher=OTHER); Wed, 09 Jan 2013 12:58:09 -0800 (PST) Message-ID: <50EDD9DC.4010107@gmail.com> Date: Wed, 09 Jan 2013 21:58:04 +0100 From: =?UTF-8?B?VmxhZGltaXIgJ8+GLWNvZGVyL3BoY29kZXInIFNlcmJpbmVua28=?= User-Agent: Mozilla/5.0 (X11; Linux x86_64; rv:10.0.11) Gecko/20121122 Icedove/10.0.11 MIME-Version: 1.0 To: grub-devel@gnu.org Subject: Re: [PATCH] Removing nested functions, part one of lots References: <20130101144204.GZ21216@riva.dynamic.greenend.org.uk> In-Reply-To: <20130101144204.GZ21216@riva.dynamic.greenend.org.uk> X-Enigmail-Version: 1.4.1 Content-Type: multipart/signed; micalg=pgp-sha512; protocol="application/pgp-signature"; boundary="------------enig4300118BB5F6D406A17F060B" X-detected-operating-system: by eggs.gnu.org: GNU/Linux 3.x [fuzzy] X-Received-From: 209.85.212.176 X-BeenThere: grub-devel@gnu.org X-Mailman-Version: 2.1.14 Precedence: list Reply-To: The development of GNU GRUB List-Id: The development of GNU GRUB List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 09 Jan 2013 20:58:18 -0000 This is an OpenPGP/MIME signed message (RFC 2440 and 3156) --------------enig4300118BB5F6D406A17F060B Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: quoted-printable Looks like nobody objects and I'm fine with this patch. Go ahead. On 01.01.2013 15:42, Colin Watson wrote: > (Part zero was a patch I already committed that dealt with some trivial= > cases.) >=20 > 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 Pave= l > nearly four years ago: >=20 > * The trampolines used when taking the address of a nested function ar= e > 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. >=20 > * Even several years on, we have failed to solve all the build system > problems associated with nested functions. See Savannah bugs #36669= > and #37938. >=20 > * 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. >=20 > 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: >=20 > * Whenever a function (usually *_iterate) takes a hook function pointe= r > "foo" as a parameter, add an additional "void *foo_data" parameter > immediately after it. >=20 > * If there is not already a typedef for the hook function type, add > one. >=20 > * Update all implementations of the hook type in question to take an > additional "void *data" parameter. >=20 > * Move each hook that was previously a nested function to the top leve= l > of its file, and mark it static. >=20 > * 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).= >=20 > * 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 =3D data"). >=20 > * If a hook requires more than one local variable from its parent > function, declare "struct _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. >=20 > * 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 "_iter". >=20 > * Remove any NESTED_FUNC_ATTR from hook declarations, and from any > pre-existing typedef for the hook function type. >=20 > 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 tha= t > 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 an= d > it makes it reasonably easy to grep for missing changes. >=20 > Please review. >=20 > Thanks, >=20 >=20 >=20 >=20 > _______________________________________________ > Grub-devel mailing list > Grub-devel@gnu.org > https://lists.gnu.org/mailman/listinfo/grub-devel --=20 Regards Vladimir '=CF=86-coder/phcoder' Serbinenko --------------enig4300118BB5F6D406A17F060B Content-Type: application/pgp-signature; name="signature.asc" Content-Description: OpenPGP digital signature Content-Disposition: attachment; filename="signature.asc" -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.12 (GNU/Linux) Comment: Using GnuPG with Mozilla - http://enigmail.mozdev.org/ iF4EAREKAAYFAlDt2dwACgkQNak7dOguQgm1igD/RoHUZVCNLK+5N8/gLxMX0WWy tF7+SVtsVQIGu+8jxFsBALWb+UEADWxwLRBwhN21fxhxu+rli818Mvy01r2gcprl =8cE6 -----END PGP SIGNATURE----- --------------enig4300118BB5F6D406A17F060B--