Re: [PATCH 1/1] EmbeddedPkg: fix gcc build errors in AndroidBootImgLib

["Laszlo Ersek" <lersek@redhat.com>]

On 07/21/20 14:50, Leif Lindholm wrote:
> Commit dbd546a32d5a
> ("BaseTools: Add gcc flag to warn on void* pointer arithmetic")
> does its work and triggers build errors in this library.
> Update the affected code to build correctly again.
> 
> Cc: Pierre Gondois <pierre.gondois@arm.com>
> Cc: Laszlo Ersek <lersek@redhat.com>
> Cc: Bob Feng<bob.c.feng@intel.com>
> Signed-off-by: Leif Lindholm <leif@nuviainc.com>
> ---
> 
> Pierre - can you please ensure to CC Arm maintainers when proposing
> changes to Arm build flags? (And build test all the top-level edk2
> packages *cough*.)

or we could perhaps introduce

  EmbeddedPkg/EmbeddedPkg.ci.yaml

> 
> Bob - can you please ensure Arm maintainers have commented on changes to
> global build flags?
> (Would it be possible to break up tools_def.template into separate
> arch-specific include files so we could have GetMaintainer.py be
> more helpful for this?)
> 
> Laszlo - you're not formally an EmbeddedPkg reviewer, but Ard is out for
> another couple of weeks. But since the Linaro CI is currently broken and
> the fix is trivial, could you have a look please?
> 
>  EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> index e1036954ee58..15b5bf451330 100644
> --- a/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> +++ b/EmbeddedPkg/Library/AndroidBootImgLib/AndroidBootImgLib.c
> @@ -97,7 +97,7 @@ AndroidBootImgGetKernelInfo (
>    ASSERT (IS_VALID_ANDROID_PAGE_SIZE (Header->PageSize));
>  
>    *KernelSize = Header->KernelSize;
> -  *Kernel = BootImg + Header->PageSize;
> +  *Kernel = (VOID *)((UINTN)BootImg + Header->PageSize);
>    return EFI_SUCCESS;
>  }
>  

Header->PageSize has type UINT32, so this is OK (the addition is
performed in UINTN, so the conversion back to VOID* is from UINTN).

> @@ -341,7 +341,7 @@ AndroidBootImgUpdateFdt (
>  
>    Status = AndroidBootImgSetProperty64 (UpdatedFdtBase, ChosenNode,
>                                          "linux,initrd-end",
> -                                        (UINTN)(RamdiskData + RamdiskSize));
> +                                        ((UINTN)RamdiskData + RamdiskSize));
>    if (EFI_ERROR (Status)) {
>      goto Fdt_Exit;
>    }
> 

RamdiskSize is a UINTN, so this is OK too.

(You could even strip the outer parentheses:

  (UINTN)RamdiskData + RamdiskSize
)

Reviewed-by: Laszlo Ersek <lersek@redhat.com>

Thanks
Laszlo