Re: [PATCH v7 00/10] Add basic Boot Loader Interface support

[Oliver Steffen]

Quoting Daniel Kiper (2023-05-19 16:19:28)
> On Wed, May 03, 2023 at 03:50:09PM +0200, Oliver Steffen wrote:
> > This is a step towards supporting unified kernel images (UKI) in Grub.
[...]
> Sadly there are still some problems with this patch set... :-(

Sorry about that...

> First of all please do rebase on latest master because there are some 
> conflicts
> due to recently merged LoongArch support.
Done.
>
> Please add more missing static keywords to the GUID assignments in the patch 
> #2, e.g.:
>   @@ -139,12 +139,12 @@ grub_cmd_lssal (struct grub_command *cmd 
> __attribute__ ((unused)),
>      const grub_efi_system_table_t *st = grub_efi_system_table;
>      grub_efi_configuration_table_t *t = st->configuration_table;
>      unsigned int i;
>   -  grub_efi_packed_guid_t guid = GRUB_EFI_SAL_TABLE_GUID;
>   +  grub_guid_t guid = GRUB_EFI_SAL_TABLE_GUID;
Done.

> Please do s/cannot opening/cannot open/ in the patch #8.
Done.
>
> The build for arm-linux-gnueabihf target complains:
>   arm-linux-gnueabihf-gcc
>   kern/misc.c: In function ‘grub_vsnprintf_real’:
>   kern/misc.c:1063:28: error: cast to pointer from integer of different size 
> [-Werror=int-to-pointer-cast]
>           grub_guid_t *guid = (grub_guid_t *) curarg;
>
> I think s/(grub_guid_t *) curarg/(grub_guid_t *)(grub_addr_t) curarg/...
Done.

> I hope you checked "pG" format specifier prints GUIDs correctly.

Yeah. I changed it a bit. The endian conversion should stay on the
caller side, and not have it in the print function. We can't make
assumptions here.
This removes the R-B on the "grub-core: Make use of guid printf format
specifier" patch, sorry.

> And last but not least, sadly you broke documentation build...
>   grub.texi:3277: warning: node next `Environment' in menu `Commands' and in 
> sectioning `Modules' differ
>   grub.texi:3893: warning: unreferenced node `Modules'
>   grub.texi:3893: warning: node `Commands' is next for `Modules' in 
> sectioning but not in menu
>   grub.texi:3893: warning: node `Environment' is prev for `Modules' in 
> sectioning but not in menu
>   grub.texi:3893: warning: node `Top' is up for `Modules' in sectioning but 
> not in menu
>   grub.texi:69: node `Top' lacks menu item for `Modules' despite being its Up 
> target
>   grub.texi:3916: warning: node prev `Commands' in menu `Environment' and in 
> sectioning `Modules' differ
>
How do you build the documentation? I don't get any warnings or errors
somehow. But it is correct, the new section was not listed in the menu.
I changed it and hope it fixes it.

> Daniel
>

  Oliver