On Thu, 31 Aug 2023 20:36:43 +0200 "Vladimir 'phcoder' Serbinenko" <phco...@gmail.com> wrote:
> I see little value in using enum here and too much compiler variance about > how it's handled. Are you specifically talking about GCC, or the diversity of compilers that GRUB might be compiled with? > Compiler is pretty much allowed to choose any type for enum. If you're talking about GCC, then with the packed attribute this statement seems false[1]. > If we go this route at all we should at very least do a compile time > assert. Sounds like a good idea. > Ideally I would keep it as-is. C unlike C++ does nothing to enforce > enum. Yes, this is unfortunate. I made this change because I think it looks nicer. I was thinking that the compiler might do obvious checking to fail if say a literal value outside the enum range is passed as a function argument that is an enum type. But you're right it doesn't do that. Using the enum type then obscures the fact that the boolean type should be 8 bits as specified by the spec. So typedef'ing as a char, as is currently done, seems to be a clearer implementation. I was also hoping that this change would provide a little more detail for backtraces in GDB, but I've yet to check this. > Feel free to put GRUB_EFI_TRUE and FALSE into unnamed enum Yes, I do like this instead of the macros. Now I'm questioning if its worth the change though. [1] https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html > Le lun. 14 août 2023, 23:39, Glenn Washburn <developm...@efficientek.com> a > écrit : > > > This allows using GRUB_EFI_TRUE and GRUB_EFI_FALSE with proper type and > > value checking. The UEFI 2.10 specification, in section 2.3.1, table 2.3, > > says the size of the boolean is 1 byte and may only contain the values 0 or > > 1. In order to have the enum be 1-byte in size instead of the default > > int-sized, add the packed attribute to the enum. > > > > Signed-off-by: Glenn Washburn <developm...@efficientek.com> > > --- > > include/grub/efi/api.h | 8 +++++++- > > 1 file changed, 7 insertions(+), 1 deletion(-) > > > > diff --git a/include/grub/efi/api.h b/include/grub/efi/api.h > > index 5934db1c992b..be7c128dfb42 100644 > > --- a/include/grub/efi/api.h > > +++ b/include/grub/efi/api.h > > @@ -552,7 +552,13 @@ enum grub_efi_reset_type > > typedef enum grub_efi_reset_type grub_efi_reset_type_t; > > > > /* Types. */ > > -typedef char grub_efi_boolean_t; > > +enum GRUB_PACKED grub_efi_boolean > > + { > > + GRUB_EFI_FALSE, > > + GRUB_EFI_TRUE > > + }; > > +typedef enum grub_efi_boolean grub_efi_boolean_t; > > + > > #if GRUB_CPU_SIZEOF_VOID_P == 8 > > typedef grub_int64_t grub_efi_intn_t; > > typedef grub_uint64_t grub_efi_uintn_t; > > -- > > 2.34.1 > > > > > > _______________________________________________ > > Grub-devel mailing list > > Grub-devel@gnu.org > > https://lists.gnu.org/mailman/listinfo/grub-devel > > _______________________________________________ Grub-devel mailing list Grub-devel@gnu.org https://lists.gnu.org/mailman/listinfo/grub-devel