On Fri, Mar 09, 2018 at 08:47:19AM +0100, Ingo Molnar wrote: > * Ard Biesheuvel <ard.biesheu...@linaro.org> wrote: > > From: Colin Ian King <colin.k...@canonical.com> > > > > 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.k...@canonical.com> > > Signed-off-by: Ard Biesheuvel <ard.biesheu...@linaro.org> > > --- > > 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