On Fri, Oct 11, 2019 at 11:04:40AM -0700, Eric Biggers wrote:
> > +struct blake2b_param
> > +{
>
> It should be 'struct blake2b_param {'
>
> checkpatch.pl should warn about this. Can you fix the checkpatch warnings
> that
> make sense to fix?
I fixed all the { at the end of line of struct definitions and left all
of type
ERROR: space prohibited after that open square bracket '['
#276: FILE: crypto/blake2b_generic.c:200:
+ v[ 9] = blake2b_IV[1];
and
WARNING: line over 80 characters
#304: FILE: crypto/blake2b_generic.c:228:
+static void blake2b_update(struct blake2b_state *S, const void *pin, size_t
inlen)
ie. where the ) or ); is beyond 80 but does not otherwise hurt
readability unlike forced newline and parameters on another line. This
is my prefrence in code I otherwise work on but I'll follow what's
common practice in crypto/.
> > +/* init xors IV with
> > +static int blake2b_init_param(struct blake2b_state *S,
> > + const struct blake2b_param *P)
> > +{
> > + const u8 *p = (const u8 *)(P);
> > + size_t i;
> > +
> > + blake2b_init0(S);
> > +
> > + /* IV XOR ParamBlock */
> > + for (i = 0; i < 8; ++i)
> > + S->h[i] ^= get_unaligned_le64(p + sizeof(S->h[i]) * i);
> > +
> > + S->outlen = P->digest_length;
> > + return 0;
> > +}
>
> No need for this to have a return value anymore. Same with:
>
> blake2b_init_param()
> blake2b_update()
> blake2b_init()
> blake2b_init_key()
> blake2b_final()
>
> The code would be more readable if they returned void, since otherwise it
> gives
> the impression that errors can occur.
Make sense.
> > +static int blake2b_update(struct blake2b_state *S, const void *pin, size_t
> > inlen)
> > +{
> > + const unsigned char *in = (const unsigned char *)pin;
>
> Convention is to use 'u8', not 'unsigned char'.
Fixed.
> > +MODULE_ALIAS_CRYPTO("blake2b");
> > +MODULE_ALIAS_CRYPTO("blake2b-generic");
>
> Should remove these module aliases now that the "blake2b" algorithm was
> removed.
A bit more of typing 'modprobe blake2-512' but I have checked if other
modules use a bare algo name as an alias and found none. So for
consistency I'll remove the 2 lines.
