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.