Re: [PATCH edk2-staging 09/19] IntelUndiPkg/GigUndiDxe: add missing UINT8* cast

[Ryszard Knop <ryszard.knop@linux.intel.com>]

On Tue, 2018-11-06 at 21:31 +0100, philmd at redhat.com wrote:
> Hi Ard,
> 
> On 6/11/18 18:58, Ard Biesheuvel wrote:
> > UINT8 and CHAR8 are not the same underlying type on all
> > architectures,
> > so add an explicit cast where necessary.
> > 
> > Contributed-under: TianoCore Contribution Agreement 1.1
> > Signed-off-by: Ard Biesheuvel <ard.biesheuvel at linaro.org>
> > ---
> >   IntelUndiPkg/GigUndiDxe/Hii.c | 2 +-
> >   1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/IntelUndiPkg/GigUndiDxe/Hii.c
> > b/IntelUndiPkg/GigUndiDxe/Hii.c
> > index a5d8ae207819..737a59fbbbac 100644
> > --- a/IntelUndiPkg/GigUndiDxe/Hii.c
> > +++ b/IntelUndiPkg/GigUndiDxe/Hii.c
> > @@ -817,7 +817,7 @@ HiiSetMenuStrings (
> >   
> >         Status = ReadPbaString (
> >                    &UndiPrivateData->NicInfo,
> > -                 PBAString8,
> > +                 (UINT8 *)PBAString8,
> >                    MAX_PBA_STR_LENGTH
> >                  );
> >         if (Status == EFI_SUCCESS) {
> > 
> 
> I'm not sure why ReadPbaString() takes UINT8* instead of CHAR8*.
> Having the device part number stored into a CHAR8[] seems correct,
> what 
> do you think?
I agree that it should use CHAR8, but the underlying code in
e1000_nvm.c is shared between multiple drivers expecting uint8_t or
equivalent, so as Ard said it'd just move the problem elsewhere. It'd
be nice to keep this conversion in ReadPbaString, but as this is the
only place where this function is used, it's fine for now. Hii.c and
friends overall need some solid refactoring if time allows.