[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v7 17/17] luks2: Use grub_log2ull to calculate log_sector_siz
From: |
Patrick Steinhardt |
Subject: |
Re: [PATCH v7 17/17] luks2: Use grub_log2ull to calculate log_sector_size and improve readability |
Date: |
Sun, 6 Dec 2020 20:44:48 +0100 |
On Fri, Dec 04, 2020 at 10:43:46AM -0600, Glenn Washburn wrote:
> Reviewed-by: Daniel Kiper <daniel.kiper@oracle.com>
> Signed-off-by: Glenn Washburn <development@efficientek.com>
> ---
> grub-core/disk/luks2.c | 3 +--
> 1 file changed, 1 insertion(+), 2 deletions(-)
>
> diff --git a/grub-core/disk/luks2.c b/grub-core/disk/luks2.c
> index 2335ded77..4fa5a0dbc 100644
> --- a/grub-core/disk/luks2.c
> +++ b/grub-core/disk/luks2.c
> @@ -642,8 +642,7 @@ luks2_recover_key (grub_disk_t source,
>
> /* Set up disk according to keyslot's segment. */
> crypt->offset_sectors = grub_divmod64 (segment.offset,
> segment.sector_size, NULL);
> - crypt->log_sector_size = sizeof (unsigned int) * 8
> - - __builtin_clz ((unsigned int) segment.sector_size) - 1;
> + crypt->log_sector_size = grub_log2ull (segment.sector_size);
I was quite confused by the fact that previously we were using an
unsigned int (which typically is 4 bytes on 64 bit systems), but now
we're using the 8 byte wide uint64. Until I realized that
`grub_log2ull(n)` uses `__builtin_clzll` instead of `__builtin_clz`. A
commit message would've helped to clear up this confusion.
Other than that:
Reviewed-by: Patrick Steinhardt <ps@pks.im>
> /* Set to the source disk size, which is the maximum we allow. */
> max_crypt_sectors = grub_disk_convert_sector(source,
> source->total_sectors,
> --
> 2.27.0
>
signature.asc
Description: PGP signature
[PATCH v7 14/17] whitespace: convert 8 spaces to tabs, Glenn Washburn, 2020/12/04