Re: [PATCH v4 1/3] multiboot2: parse vga= option when setting GOP mode

[Jan Beulich <jbeulich@xxxxxxxx>]

On 07.07.2023 12:13, Roger Pau Monné wrote:
> On Thu, Jul 06, 2023 at 12:41:58PM +0200, Jan Beulich wrote:
>> On 05.07.2023 13:47, Roger Pau Monne wrote:
>>> --- a/xen/arch/x86/efi/efi-boot.h
>>> +++ b/xen/arch/x86/efi/efi-boot.h
>>> @@ -795,7 +795,30 @@ static bool __init 
>>> efi_arch_use_config_file(EFI_SYSTEM_TABLE *SystemTable)
>>>  
>>>  static void __init efi_arch_flush_dcache_area(const void *vaddr, UINTN 
>>> size) { }
>>>  
>>> -void __init efi_multiboot2(EFI_HANDLE ImageHandle, EFI_SYSTEM_TABLE 
>>> *SystemTable)
>>> +/* Return a pointer to the character after the first occurrence of opt in 
>>> cmd */
>>> +static const char __init *get_option(const char *cmd, const char *opt)
>>
>> Nit: __init and * want to change places.
> 
> Hm, yes.  I assume that placing it before the return type is not OK?
> (static const __init char ...)

That's still in the middle of the return type then. Technically gcc
accepts it being placed anywhere, but they reserve the right to change
meaning when not placed appropriately. Recall that you may alter both
attributes of a function (or variable) and attributes of types. Hence
to disambiguate both, proper placement may become necessary down the
road. And while it might be that

static __init const char *...

would also be okay-ish (albeit I'm not certain), that's still against
how we do things commonly (i.e. a not written down style aspect).

>>>[...]
> 
> LGTM, please adjust if you don't mind, otherwise I can send an
> adjusted version.

No need to send an update.

Jan