grub-devel
[Top][All Lists]
Advanced

[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
> 

Attachment: signature.asc
Description: PGP signature


reply via email to

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