grub-devel
[Top][All Lists]
Advanced

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: [PATCH v6 04/12] cryptodisk: Replace some literals with constants in


From: Daniel Kiper
Subject: Re: [PATCH v6 04/12] cryptodisk: Replace some literals with constants in grub_cryptodisk_endecrypt
Date: Wed, 2 Dec 2020 18:37:42 +0100
User-agent: NeoMutt/20170113 (1.7.2)

On Fri, Nov 27, 2020 at 03:03:36AM -0600, Glenn Washburn wrote:
> This should improve readability of code by providing clues as to what the
> value represents. The new macro GRUB_TYPE_BITS(type) returns the number of
> bits allocated for type. Also add GRUB_TYPE_U_MAX/MIN(type) macros to get
> the max/min values for an unsigned number with size of type.

Two separate patches please...

> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
>  grub-core/disk/cryptodisk.c | 13 +++++++------
>  include/grub/types.h        |  5 +++++
>  2 files changed, 12 insertions(+), 6 deletions(-)
>
> diff --git a/grub-core/disk/cryptodisk.c b/grub-core/disk/cryptodisk.c
> index 473c93976..31b73c535 100644
> --- a/grub-core/disk/cryptodisk.c
> +++ b/grub-core/disk/cryptodisk.c
> @@ -284,22 +284,23 @@ grub_cryptodisk_endecrypt (struct grub_cryptodisk *dev,
>         iv[1] = grub_cpu_to_le32 (sector >> 32);
>         /* FALLTHROUGH */
>       case GRUB_CRYPTODISK_MODE_IV_PLAIN:
> -       iv[0] = grub_cpu_to_le32 (sector & 0xFFFFFFFF);
> +       iv[0] = grub_cpu_to_le32 (sector & GRUB_TYPE_U_MAX (iv[0]));
>         break;
>       case GRUB_CRYPTODISK_MODE_IV_BYTECOUNT64:
> -       iv[1] = grub_cpu_to_le32 (sector >> (32 - dev->log_sector_size));
> +       iv[1] = grub_cpu_to_le32 (sector >> (GRUB_TYPE_BITS (iv[1])
> +                                            - dev->log_sector_size));
>         iv[0] = grub_cpu_to_le32 ((sector << dev->log_sector_size)
> -                                 & 0xFFFFFFFF);
> +                                 & GRUB_TYPE_U_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_BITS (iv[0]));
> +         iv[sz - 1] = grub_cpu_to_be32 (num & GRUB_TYPE_U_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_U_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 f22055f98..29d807f71 100644
> --- a/include/grub/types.h
> +++ b/include/grub/types.h
> @@ -72,6 +72,8 @@
>  # endif
>  #endif
>
> +#define GRUB_TYPE_BITS(type) (sizeof(type) * GRUB_CHAR_BIT)

This macro should go below "#ifndef __CHAR_BIT__ ...".

>  #ifndef __CHAR_BIT__
>  # error __CHAR_BIT__ is not defined
>  #elif __CHAR_BIT__ != 8
> @@ -159,6 +161,9 @@ typedef grub_int32_t      grub_ssize_t;
>  #endif
>  # define GRUB_LONG_MIN (-GRUB_LONG_MAX - 1)
>
> +#define GRUB_TYPE_U_MAX(type) ((typeof (1ULL))((typeof (type))(~0)))

(typeof (1ULL)) == (unsigned long long)?

> +#define GRUB_TYPE_U_MIN(type) 0ULL

I am not sure why you cast everything to ULL/unsigned long long. Should
not the final cast be to (typeof (type))?

Daniel



reply via email to

[Prev in Thread] Current Thread [Next in Thread]