[12/12] efi: make const array 'apple' static

Message ID 20180308080020.22828-13-ard.biesheuvel@linaro.org
State New
Headers show
Series
  • first batch of EFI changes for v4.17
Related show

Commit Message

Ard Biesheuvel March 8, 2018, 8 a.m.
From: Colin Ian King <colin.king@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.king@canonical.com>

Signed-off-by: Ard Biesheuvel <ard.biesheuvel@linaro.org>

---
 arch/x86/boot/compressed/eboot.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

-- 
2.15.1

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Comments

Joe Perches March 8, 2018, 11:05 a.m. | #1
On Thu, 2018-03-08 at 08:00 +0000, Ard Biesheuvel wrote:
> From: Colin Ian King <colin.king@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.king@canonical.com>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 };


Perhaps 

static const efi_char16_t apple[] ...

is better.

>  	efi_char16_t *fw_vendor = (efi_char16_t *)(unsigned long)

>  		efi_table_attr(efi_system_table, fw_vendor, sys_table);

>  

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel March 9, 2018, 7:43 a.m. | #2
On 8 March 2018 at 11:05, Joe Perches <joe@perches.com> wrote:
> On Thu, 2018-03-08 at 08:00 +0000, Ard Biesheuvel wrote:

>> From: Colin Ian King <colin.king@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.king@canonical.com>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 };

>

> Perhaps

>

> static const efi_char16_t apple[] ...

>

> is better.

>


Why would that be any better? I have always found the 'const'
placement after the type to be much clearer.

const void *
void const *
void * const

I.e., #2 and #3 are equivalent, and so 'const' associates to the left
not to the right, unless it is at the beginning.

Personally, I don't mind either way, but saying it is 'better' is a stretch imo
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel March 9, 2018, 7:44 a.m. | #3
On 9 March 2018 at 07:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 8 March 2018 at 11:05, Joe Perches <joe@perches.com> wrote:

>> On Thu, 2018-03-08 at 08:00 +0000, Ard Biesheuvel wrote:

>>> From: Colin Ian King <colin.king@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.king@canonical.com>

>>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 };

>>

>> Perhaps

>>

>> static const efi_char16_t apple[] ...

>>

>> is better.

>>

>

> Why would that be any better? I have always found the 'const'

> placement after the type to be much clearer.

>

> const void *

> void const *

> void * const

>

> I.e., #2 and #3 are equivalent,


That would be #1 and #2, of course

> and so 'const' associates to the left

> not to the right, unless it is at the beginning.

>

> Personally, I don't mind either way, but saying it is 'better' is a stretch imo

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar March 9, 2018, 7:47 a.m. | #4
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> From: Colin Ian King <colin.king@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.king@canonical.com>

> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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.

This makes it abundantly clear that it's not on the stack.

Also, would it make sense to rename it to something more descriptive like 
"apple_unicode_str[]" or so?

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?

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel March 9, 2018, 7:52 a.m. | #5
On 9 March 2018 at 07:47, Ingo Molnar <mingo@kernel.org> wrote:
>

> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>

>> From: Colin Ian King <colin.king@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.king@canonical.com>

>> Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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.

>

> This makes it abundantly clear that it's not on the stack.

>


Fair enough. I didn't know there was such a policy, but since these
have local scope by definition, it doesn't pollute the global
namespace so it's fine

> Also, would it make sense to rename it to something more descriptive like

> "apple_unicode_str[]" or so?

>

> 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?

>


is u"xxx" the same as L"xxx"?

In any case, this is for historical reasons: at some point (and I
don't remember the exact details) we had a conflict at link time with
objects using 4 byte wchar_t, so we started using this notation to be
independent of the size of wchar_t. That issue no longer exists so we
should be able to get rid of this.
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar March 9, 2018, 8:04 a.m. | #6
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> > Also, would it make sense to rename it to something more descriptive like

> > "apple_unicode_str[]" or so?

> >

> > 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?

> >

> 

> is u"xxx" the same as L"xxx"?


So "L" literals map to wchar_t, which wide character type is implementation 
specific IIRC, could be 16-bit or 32-bit wide.

u"" literals OTOH are specified by the C11 spec to be char16_t, i.e. 16-bit wide 
characters - which I assume is the EFI type as well?

> In any case, this is for historical reasons: at some point (and I

> don't remember the exact details) we had a conflict at link time with

> objects using 4 byte wchar_t, so we started using this notation to be

> independent of the size of wchar_t. That issue no longer exists so we

> should be able to get rid of this.


Yes, my guess is that those problems were due to L"xyz" mapping to wchar_t and 
having a different type in the kernel build and the host build side - but u"xyz" 
should solve that.

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ard Biesheuvel March 9, 2018, 8:19 a.m. | #7
On 9 March 2018 at 08:07, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:
> On 9 March 2018 at 08:04, Ingo Molnar <mingo@kernel.org> wrote:

>>

>> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

>>

>>> > Also, would it make sense to rename it to something more descriptive like

>>> > "apple_unicode_str[]" or so?

>>> >

>>> > 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?

>>> >

>>>

>>> is u"xxx" the same as L"xxx"?

>>

>> So "L" literals map to wchar_t, which wide character type is implementation

>> specific IIRC, could be 16-bit or 32-bit wide.

>>

>> u"" literals OTOH are specified by the C11 spec to be char16_t, i.e. 16-bit wide

>> characters - which I assume is the EFI type as well?

>>

>>> In any case, this is for historical reasons: at some point (and I

>>> don't remember the exact details) we had a conflict at link time with

>>> objects using 4 byte wchar_t, so we started using this notation to be

>>> independent of the size of wchar_t. That issue no longer exists so we

>>> should be able to get rid of this.

>>

>> Yes, my guess is that those problems were due to L"xyz" mapping to wchar_t and

>> having a different type in the kernel build and the host build side - but u"xyz"

>> should solve that.

>>

>

> Excellent!

>

> Do you mind taking this patch as is? I will follow up with a patch

> that updates all occurrences of this pattern (we have a few of them),

> i.e., use u"" notation and move them to file scope.


OK, I misremembered: the other occurrences have already been moved to
file scope a while ago.

I will follow up with a patch that switches to u"" notation, please
let me know if I should respin this or not
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Lukas Wunner March 9, 2018, 8:29 a.m. | #8
On Fri, Mar 09, 2018 at 08:47:19AM +0100, Ingo Molnar wrote:
> * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> > From: Colin Ian King <colin.king@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.king@canonical.com>

> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Ingo Molnar March 9, 2018, 8:31 a.m. | #9
* Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> On 9 March 2018 at 08:04, Ingo Molnar <mingo@kernel.org> wrote:

> >

> > * Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> >

> >> > Also, would it make sense to rename it to something more descriptive like

> >> > "apple_unicode_str[]" or so?

> >> >

> >> > 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?

> >> >

> >>

> >> is u"xxx" the same as L"xxx"?

> >

> > So "L" literals map to wchar_t, which wide character type is implementation

> > specific IIRC, could be 16-bit or 32-bit wide.

> >

> > u"" literals OTOH are specified by the C11 spec to be char16_t, i.e. 16-bit wide

> > characters - which I assume is the EFI type as well?

> >

> >> In any case, this is for historical reasons: at some point (and I

> >> don't remember the exact details) we had a conflict at link time with

> >> objects using 4 byte wchar_t, so we started using this notation to be

> >> independent of the size of wchar_t. That issue no longer exists so we

> >> should be able to get rid of this.

> >

> > Yes, my guess is that those problems were due to L"xyz" mapping to wchar_t and

> > having a different type in the kernel build and the host build side - but u"xyz"

> > should solve that.

> >

> 

> Excellent!


Please double check the generated code though, all of this is from memory.

> Do you mind taking this patch as is? I will follow up with a patch

> that updates all occurrences of this pattern (we have a few of them),

> i.e., use u"" notation and move them to file scope.


Sure, done!

Thanks,

	Ingo
--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Joe Perches March 9, 2018, 9:37 a.m. | #10
On Fri, 2018-03-09 at 07:44 +0000, Ard Biesheuvel wrote:
> On 9 March 2018 at 07:43, Ard Biesheuvel <ard.biesheuvel@linaro.org> wrote:

> > On 8 March 2018 at 11:05, Joe Perches <joe@perches.com> wrote:

> > > On Thu, 2018-03-08 at 08:00 +0000, Ard Biesheuvel wrote:

> > > > From: Colin Ian King <colin.king@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.king@canonical.com>

> > > > Signed-off-by: Ard Biesheuvel <ard.biesheuvel@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 };

> > > 

> > > Perhaps

> > > 

> > > static const efi_char16_t apple[] ...

> > > 

> > > is better.

> > > 

> > 

> > Why would that be any better?


It'd be more like the the style used
in the rest of the kernel.

--
To unsubscribe from this list: send the line "unsubscribe linux-efi" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Patch

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);