On Fri, Mar 09, 2018 at 08:47:19AM +0100, Ingo Molnar wrote:
> * Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx> wrote:
> > From: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> >
> > Don't populate the const read-only array 'buf' on the stack but instead
> > make it static. Makes the object code smaller by 64 bytes:
> >
> > Before:
> > text data bss dec hex filename
> > 9264 1 16 9281 2441 arch/x86/boot/compressed/eboot.o
> >
> > After:
> > text data bss dec hex filename
> > 9200 1 16 9217 2401 arch/x86/boot/compressed/eboot.o
> >
> > (gcc version 7.2.0 x86_64)
> >
> > Signed-off-by: Colin Ian King <colin.king@xxxxxxxxxxxxx>
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@xxxxxxxxxx>
> > ---
> > arch/x86/boot/compressed/eboot.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/arch/x86/boot/compressed/eboot.c b/arch/x86/boot/compressed/eboot.c
> > index 886a9115af62..f2251c1c9853 100644
> > --- a/arch/x86/boot/compressed/eboot.c
> > +++ b/arch/x86/boot/compressed/eboot.c
> > @@ -423,7 +423,7 @@ static void retrieve_apple_device_properties(struct boot_params *boot_params)
> >
> > static void setup_quirks(struct boot_params *boot_params)
> > {
> > - efi_char16_t const apple[] = { 'A', 'p', 'p', 'l', 'e', 0 };
> > + static efi_char16_t const apple[] = { 'A', 'p', 'p', 'l', 'e', 0 };
> > efi_char16_t *fw_vendor = (efi_char16_t *)(unsigned long)
> > efi_table_attr(efi_system_table, fw_vendor, sys_table);
>
> As a general policy, please don't put 'static' variables into the local
> scope, use file scope instead - right before setup_quirks() would be fine.
Well, I believe the end result is the same and the closer the declaration
is to where it's used, the easier the code is to read and understand.
I object to patches like this because they paper over a missing
compiler optimization:
https://gcc.gnu.org/bugzilla/show_bug.cgi?id=68725
I have told Colin before that it would be more useful to look into
fixing the underlying compiler issue rather than polluting the kernel
with "static" keywords, but he keeps sending these patches so I've
given up responding:
https://lkml.org/lkml/2017/8/25/636
> Plus an unicode string literal initializer would be pretty descriptive
> as well, instead of the weird looking character array, i.e. something
> like:
>
> static efi_char16_t const apple_unicode_str[] = u"Apple";
>
> ... or so?
Last time I checked this didn't work, I believe it's because it's C11
and the kernel is compiled with -std=gnu89. And I'm also not sure if
the oldest gcc version that we support understands u"".
Thanks,
Lukas
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
