[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in
From: |
Patrick Steinhardt |
Subject: |
Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt. |
Date: |
Wed, 28 Oct 2020 19:16:56 +0100 |
On Mon, Oct 26, 2020 at 06:25:53PM +0000, Glenn Washburn wrote:
> Oct 23, 2020 1:00:59 PM Patrick Steinhardt <ps@pks.im>:
>
> > On Mon, Oct 19, 2020 at 06:09:55PM -0500, Glenn Washburn wrote:
> >> This should improve readability of code by providing clues as to what the
> >> value represents.
> >>
> >> Signed-off-by: Glenn Washburn <development@efficientek.com>
> >> ---
> >> grub-core/disk/cryptodisk.c | 12 +++++++-----
> >> include/grub/types.h | 3 +++
> >> 2 files changed, 10 insertions(+), 5 deletions(-)
> >>
> >> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> >> index 623f0f396..1a91c2d55 100644
> >> --- a/grub-core/disk/cryptodisk.c
> >> +++ b/grub-core/disk/cryptodisk.c
> >> @@ -297,19 +297,21 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk
> >> *dev,
> >> }
> >> break;
> >> case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> >> - iv[1] = grub_cpu_to_le32 (sector >> (32 - log_sector_size));
> >> + /* The IV is the 64 bit byte offset of the sector. */
> >> + iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BIT(iv[0])
> >> + - log_sector_size));
> >> iv[0] = grub_cpu_to_le32 ((sector << log_sector_size)
> >> - & 0xFFFFFFFF);
> >> + & GRUB_TYPE_MAX(iv[0]));
> >> break;
> >> case GRUB_CRYPTODISK_MODE_IV_BENBI:
> >> {
> >> grub_uint64_t num = (sector << dev->benbi_log) + 1;
> >> - iv[sz - 2] = grub_cpu_to_be32 (num >> 32);
> >> - iv[sz - 1] = grub_cpu_to_be32 (num & 0xFFFFFFFF);
> >> + iv[sz - 2] = grub_cpu_to_be32 (num >> GRUB_TYPE_BIT(iv[0]));
> >> + iv[sz - 1] = grub_cpu_to_be32 (num & GRUB_TYPE_MAX(iv[0]));
> >> }
> >> break;
> >> case GRUB_CRYPTODISK_MODE_IV_ESSIV:
> >> - iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
> >> + iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_MAX(iv[0]));
> >> err = grub_crypto_ecb_encrypt (dev->essiv_cipher, iv, iv,
> >> dev->cipher->cipher->blocksize);
> >> if (err)
> >> diff --git a/include/grub/types.h b/include/grub/types.h
> >> index 035a4b528..8b4267ebd 100644
> >> --- a/include/grub/types.h
> >> +++ b/include/grub/types.h
> >> @@ -319,4 +319,7 @@ static inline void grub_set_unaligned64 (void *ptr,
> >> grub_uint64_t val)
> >>
> >> #define GRUB_CHAR_BIT 8
> >>
> >> +#define GRUB_TYPE_BIT(type) (sizeof(type) * GRUB_CHAR_BIT)
> >
> > Hum. I'd rather name this `GRUB_TYPE_BITS`, as the current name implies
> > that we only get a single bit. Other than that the change looks good to
> > me and I agree that it helps readability.
> >
> > Patrick
>
> Yes, I would've liked to also, but I decided to stick with the naming
> convention as can be seen in the definition of GRUB_CHAR_BIT above it.
> Should there be a separate patch renaming GRUB_CHAR_BIT also?
Nah, your naming makes sense in that context. Let's keep it as-is in
this patch.
Patrick
signature.asc
Description: PGP signature
- Re: [PATCH v3 02/10] luks2: Improve readability in luks2_get_keyslot., (continued)
- [PATCH v3 03/10] luks2: Use more intuitive keyslot key instead of index when naming keyslot., Glenn Washburn, 2020/10/19
- Re: [PATCH v3 03/10] luks2: Use more intuitive keyslot key instead of index when naming keyslot., Patrick Steinhardt, 2020/10/23
- [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages., Glenn Washburn, 2020/10/19
- Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages., Daniel Kiper, 2020/10/23
- Re: [PATCH v3 01/10] luks2: Fix use of incorrect index and some grub_error() messages., Patrick Steinhardt, 2020/10/23
- [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Glenn Washburn, 2020/10/19
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Patrick Steinhardt, 2020/10/23
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Glenn Washburn, 2020/10/26
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt.,
Patrick Steinhardt <=
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Daniel Kiper, 2020/10/27
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Glenn Washburn, 2020/10/29
- Re: [PATCH v3 07/10] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt., Daniel Kiper, 2020/10/30
- [PATCH v3 10/10] luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c., Glenn Washburn, 2020/10/19
- Re: [PATCH v3 10/10] luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c., Patrick Steinhardt, 2020/10/23
- Re: [PATCH v3 10/10] luks2: Rename source disk variabled named 'disk' to 'source' as in luks.c., Daniel Kiper, 2020/10/27
- [PATCH v3 04/10] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors, Glenn Washburn, 2020/10/19
- Re: [PATCH v3 04/10] luks2: grub_cryptodisk_t->total_length is the max number of device native sectors, Patrick Steinhardt, 2020/10/23
- [PATCH v3 05/10] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'., Glenn Washburn, 2020/10/19
- Re: [PATCH v3 05/10] cryptodisk: Fix cipher IV mode 'plain64' always being set as 'plain'., Daniel Kiper, 2020/10/23