2024-03-03 06:37:31

by James Bottomley

[permalink] [raw]
Subject: Re: [PATCH v4 01/12] crypto: ecdsa - Convert byte arrays with key coordinates to digits

On Sat, 2024-03-02 at 22:34 +0100, Lukas Wunner wrote:
> On Thu, Feb 29, 2024 at 09:19:56PM -0500, Stefan Berger wrote:
[...]
> > @@ -238,12 +237,17 @@ static int ecdsa_set_pub_key(struct
> > crypto_akcipher *tfm, const void *key, unsig
> >                 return -EINVAL;
> >  
> >         keylen--;
> > -       ndigits = (keylen >> 1) / sizeof(u64);
> > +       digitlen = keylen >> 1;
> > +
> > +       ndigits = DIV_ROUND_UP(digitlen, sizeof(u64));
>
> Instead of introducing an additional digitlen variable, you could
> just use keylen.  It seems it's not used in the remainder of the
> function, so modifying it is harmless:
>
>         keylen--;
> +       keylen >>= 1;
> -       ndigits = (keylen >> 1) / sizeof(u64);
> +       ndigits = DIV_ROUND_UP(digitlen, sizeof(u64));
>
> Just a suggestion.

The compiler will optimize the variables like this anyway (reuse
registers or frames after a current consumer becomes unused) so there's
no requirement to do this for efficiency, the only real question is
whether using digitlen is clearer than reusing keylen, which I'll leave
to the author.

James