Re: [PATCH edk2-platforms v2 6/7] Platform/NinetySixBoards: add core driver for LS connector and config

[Leif Lindholm <leif.lindholm@linaro.org>]

On Thu, Feb 22, 2018 at 07:10:52PM +0000, Ard Biesheuvel wrote:
> >>> +#define MEZZANINE_NONE                            0x0
> >>> +#define MEZZANINE_SECURE96                        0x1
> >>> +#define MEZZANINE_MAX                             0x1
> >>
> >> Would this be simpler as an enum with a permanent final element called
> >> MaxMezzanineType? This seems like the common EDK2 (and even UEFI)
> >> pattern.
> >>
> >
> > Does C guarantee a certain size for enums? Because the struct below is
> > projected onto the EFI variable directly.
> 
> OK, so apparently an enum is an int, unless you explicitly tell the
> compiler otherwise.

Not exactly.
Enums are compile-time resolved, so it is entirely within the
compiler's ability to handle this situation safely.

Try the following for yourself:
#include <stdio.h>

typedef enum {
  TestFirst = 255,
  TestMax
} MYTEST;

typedef struct {
  unsigned char test;
} Foo;

int main(void)
{
  Foo bar;

  bar.test = TestFirst;
  bar.test = TestMax;
  printf("bar.test: %d\n", bar.test);
}

I'd be astonished if you can find a compiler that won't give you
something similar to:
warning: large integer implicitly truncated to unsigned type [-Woverflow]
   bar.test = TestMax;

> That does mean the type field uses up 32 bits rather than 8, so I'd
> prefer to keep a UINT8 here instead.

Does the above change your opinion in any way?...

> I could still use 0x2 rather than 0x1 as the max value if you prefer.

...if not, that would be preferable, yes.

> >> I would prefer if we could get this one in with a
> >> #include <Uefi/UefiMultiPhase.h>
> >> instead.
> >>
> >
> > OK, but please chase this with the other maintainers then.

Done.

/
    Leif