Herbert Xu <herbert@xxxxxxxxxxxxxxxxxxx> wrote: > +#include <crypto/hash.h> > +#include <crypto/sm2.h> > +#include <keys/asymmetric-parser.h> > +#include <keys/asymmetric-subtype.h> > +#include <keys/system_keyring.h> > #include <linux/module.h> > #include <linux/kernel.h> > #include <linux/slab.h> > -#include <keys/asymmetric-subtype.h> > -#include <keys/asymmetric-parser.h> > -#include <keys/system_keyring.h> > -#include <crypto/hash.h> > +#include <linux/string.h> Why rearrage the order? Why not leave the linux/ headers first? Then the keys/ and then the crypto/. > + if (strcmp(cert->pub->pkey_algo, "sm2") == 0) { > + ret = strcmp(sig->hash_algo, "sm3") != 0 ? -EINVAL : > + crypto_shash_init(desc) ?: > + sm2_compute_z_digest(desc, cert->pub->key, > + cert->pub->keylen, sig->digest) ?: > + crypto_shash_init(desc) ?: > + crypto_shash_update(desc, sig->digest, > + sig->digest_size) ?: > + crypto_shash_finup(desc, cert->tbs, cert->tbs_size, > + sig->digest); Ewww... That's really quite hard to comprehend at a glance. :-) Should sm2_compute_z_digest() be something accessible through the crypto hooks rather than being called directly? > + } else "} else {" please. > + ret = crypto_shash_digest(desc, cert->tbs, cert->tbs_size, > + sig->digest); > + David