Hi, On Wed, Oct 09, 2019 at 03:47:09PM +0200, Ard Biesheuvel wrote: > I have a couple more comments - apologies for not spotting these the > first time around. No problem, there was a lot of churn since v1. > > +enum { > > + BLAKE2_DUMMY_2 = 1 / (sizeof(struct blake2b_param) == BLAKE2B_OUTBYTES) > > +}; > > + > > Please use BUILD_BUG_ON(<condition>) to do compile time sanity checks. > (You'll have to move it into a C function though) Fixed. > > +int blake2b_init_key(struct blake2b_state *S, size_t outlen, const void *key, > > This should be static, and given that it is not used anywhere, you > should either remove it or wire it up. > > Given that blake2 can be used as a keyed hash as well as an unkeyed > hash, I propose that you implement the setkey() hook, and add > CRYPTO_ALG_OPTIONAL_KEY to the cra_flags to convey that setkey() is > optional. Ok, setkey will be in v3. > > +int blake2b_init(struct blake2b_state *S, size_t outlen); > > +int blake2b_init_key(struct blake2b_state *S, size_t outlen, const void *key, size_t keylen); > > +int blake2b_update(struct blake2b_state *S, const void *in, size_t inlen); > > +int blake2b_final(struct blake2b_state *S, void *out, size_t outlen); > > Drop these please. Done, with the additional 'static' fixups.