On Thu, 31 Aug 2023 20:05:37 +0200
Daniel Kiper <dki...@net-space.pl> wrote:
> On Mon, Aug 14, 2023 at 04:38:40PM -0500, Glenn Washburn wrote:
> > 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.
>
> Hmmm... Could you point out us an official doc saying "packed" attribute
> does that?
This has been around since at least gcc 3.3, but this is link for a
more current gcc[1] (search for "packed").
> And I hope you tested that change because we use grub_efi_boolean_t type
> quite often...
I've been using this change for a couple months now with no issues.
Also, no tests are failing because of this change, although this might
not be a great recommendation. I want to do some more testing, just to
be sure.
>
> > 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,
>
> I would explicitly do
>
> GRUB_EFI_FALSE = 0,
Good idea.
>
> > + GRUB_EFI_TRUE
> > + };
>
> I would move GRUB_PACKED here to be inline with its other uses.
The following from the GCC manual has me questioning why we do this:
"You can also place [attributes] just past the closing curly brace of
the definition, but this is less preferred because logically the type
should be fully defined at the closing brace."[2] I'll change this in
the next iteration for consistency, but perhaps we should reconsider
the policy?
Glenn
[1] https://gcc.gnu.org/onlinedocs/gcc/Common-Type-Attributes.html
[2] https://gcc.gnu.org/onlinedocs/gcc/Type-Attributes.html
_______________________________________________
Grub-devel mailing list
Grub-devel@gnu.org
https://lists.gnu.org/mailman/listinfo/grub-devel
