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

[Roger Pau Monné <roger.pau@xxxxxxxxxx>]

On Fri, Jul 07, 2023 at 12:24:10PM +0200, Jan Beulich wrote:
> 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).

Thanks for the explanation.

Roger.