[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 4/5] cryptodisk: Use enum constants as indexes into crypto
From: |
Glenn Washburn |
Subject: |
Re: [PATCH v2 4/5] cryptodisk: Use enum constants as indexes into cryptomount option array |
Date: |
Thu, 19 May 2022 15:31:48 -0500 |
On Thu, 19 May 2022 20:24:11 +0200
Daniel Kiper <dkiper@net-space.pl> wrote:
> On Fri, May 13, 2022 at 12:00:50PM -0500, Glenn Washburn wrote:
> > Signed-off-by: Glenn Washburn <development@efficientek.com>
> > ---
> > grub-core/disk/cryptodisk.c | 53 ++++++++++++++++++++++---------------
> > 1 file changed, 32 insertions(+), 21 deletions(-)
> >
> > diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> > index 94640b502..ecbda7ce9 100644
> > --- a/grub-core/disk/cryptodisk.c
> > +++ b/grub-core/disk/cryptodisk.c
> > @@ -35,6 +35,17 @@ GRUB_MOD_LICENSE ("GPLv3+");
> >
> > grub_cryptodisk_dev_t grub_cryptodisk_list;
> >
> > +enum
> > + {
> > + OPTION_UUID,
> > + OPTION_ALL,
> > + OPTION_BOOT,
> > + OPTION_PASSWORD,
> > + OPTION_KEYFILE,
> > + OPTION_KEYFILE_OFFSET,
> > + OPTION_KEYFILE_SIZE
> > + };
>
> I would prefer constants here.
I chose enum because that is consistent with many other commands. By
"constants" do you mean CPP defined macros? I can imagine you would
mean variables with the "const" property. Without having done an
exhaustive search, I don't believe any commands use CPP macros for the
index. Here's a list of files for commands that use enums as the index:
grub-core/commands/i386/pc/drivemap.c
grub-core/commands/file.c
grub-core/commands/pgp.c
grub-core/commands/search_wrap.c
grub-core/term/gfxterm_background.c
grub-core/term/serial.c
grub-core/term/terminfo.c
The benefits of enum over CPP macro is that inserting into the enum
list automatically renumbers the subsequent constants in the list. I
failing to think of a good way to do that with CPP macros. Could you
explain a little further precisely what you're wanting (maybe I've
guessed wrong) and why you see it as superior to using enums?
Glenn
>
> Otherwise patches LGTM.
>
> Daniel
- [PATCH v2 0/5] Cryptomount keyfile support, Glenn Washburn, 2022/05/13
- [PATCH v2 1/5] cryptodisk: luks: Unify grub_cryptodisk_dev function names, Glenn Washburn, 2022/05/13
- [PATCH v2 2/5] cryptodisk: geli: Unify grub_cryptodisk_dev function names, Glenn Washburn, 2022/05/13
- [PATCH v2 3/5] cryptodisk: Add options to cryptomount to support keyfiles, Glenn Washburn, 2022/05/13
- [PATCH v2 5/5] docs: Add documentation on keyfile option to cryptomount, Glenn Washburn, 2022/05/13
- [PATCH v2 4/5] cryptodisk: Use enum constants as indexes into cryptomount option array, Glenn Washburn, 2022/05/13