[edk2-devel] [PATCH 1/1] EmbeddedPkg: fix gcc build errors in AndroidBootImgLib

[Leif Lindholm]

On Wed, Jul 22, 2020 at 22:34:52 +0200, Laszlo Ersek wrote:
> 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 at arm.com>
> > Cc: Laszlo Ersek <lersek at redhat.com>
> > Cc: Bob Feng<bob.c.feng at intel.com>
> > Signed-off-by: Leif Lindholm <leif at 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

Ah, do those get picked up automagically?
If so we should definitely do that, as well as for ArmPkg.

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

Yes, I'll do that.
I just found the moving a single character aesthetically pleasing :)

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

Thanks!

/
    Leif

-=-=-=-=-=-=-=-=-=-=-=-
Groups.io Links: You receive all messages sent to this group.

View/Reply Online (#63188): https://edk2.groups.io/g/devel/message/63188
Mute This Topic: https://groups.io/mt/75702678/1813853
Group Owner: devel+owner at edk2.groups.io
Unsubscribe: https://edk2.groups.io/g/devel/unsub  [edk2-devel-archive at redhat.com]
-=-=-=-=-=-=-=-=-=-=-=-