[<prev] [next>] [<thread-prev] [thread-next>] [day] [month] [year] [list]
Message-ID: <20141023122845.GP23154@mwanda>
Date: Thu, 23 Oct 2014 15:28:45 +0300
From: Dan Carpenter <dan.carpenter@...cle.com>
To: Eric Rost <eric.rost@...abylon.net>
Cc: gregkh@...uxfoundation.org, jason@...edaemon.net, jake@....net,
antonysaraev@...il.com, devel@...verdev.osuosl.org,
linux-kernel@...r.kernel.org
Subject: Re: [PATCH v2 1/2] staging: skein: Add Crypto API support
On Wed, Oct 22, 2014 at 09:23:51PM -0500, Eric Rost wrote:
> Adds crypto API support for the skein module. Also collapses the
> threefish module into the skein module.
>
Why is this in staging anyway? It seems very small and not terrible
code. It could easily be sent to the main kernel.
> diff --git a/drivers/staging/skein/skein.h b/drivers/staging/skein/skein.h
> index e6669f1..79fac00 100644
> --- a/drivers/staging/skein/skein.h
> +++ b/drivers/staging/skein/skein.h
> @@ -28,6 +28,11 @@
> **
> ***************************************************************************/
>
> +/*Skein digest sizes for crypto api*/
> +#define SKEIN256_DIGEST_BIT_SIZE (256)
> +#define SKEIN512_DIGEST_BIT_SIZE (512)
> +#define SKEIN1024_DIGEST_BIT_SIZE (1024)
Superfulous parens.
> +
> #ifndef rotl_64
> #define rotl_64(x, N) (((x) << (N)) | ((x) >> (64-(N))))
Missing spaces.
#define rotl_64(x, N) (((x) << (N)) | ((x) >> (64 - (N))))
Also this should be a function. N and x are evaulated twice so it could
be a bug depending on how it's called.
> #endif
> diff --git a/drivers/staging/skein/skein_generic.c b/drivers/staging/skein/skein_generic.c
> new file mode 100644
> index 0000000..14cc5bd
> --- /dev/null
> +++ b/drivers/staging/skein/skein_generic.c
> @@ -0,0 +1,109 @@
> +/*
> + * Cryptographic API.
> + *
> + * Skein256 Hash Algorithm.
> + *
> + * Derived from cryptoapi implementation, adapted for in-place
> + * scatterlist interface.
> + *
> + * Copyright (c) Eric Rost <eric.rost@...abylon.net>
> + *
> + * This program is free software; you can redistribute it and/or modify it
> + * under the terms of the GNU General Public License as published by the Free
> + * Software Foundation; either version 2 of the License, or (at your option)
> + * any later version.
> + *
> + */
> +#include <crypto/internal/hash.h>
> +#include <linux/init.h>
> +#include <linux/mm.h>
> +#include <linux/cryptohash.h>
> +#include <linux/types.h>
> +#include "skein.h"
> +#include <asm/byteorder.h>
> +
> +static int skein256_init(struct shash_desc *desc)
> +{
> + struct skein_256_ctx *sctx = shash_desc_ctx(desc);
> +
Don't put a blank line in the middle of the declaration block.
> + int ret = skein_256_init(sctx, SKEIN256_DIGEST_BIT_SIZE);
> +
> + return ret;
> +}
This should be:
static int skein256_init(struct shash_desc *desc)
{
struct skein_256_ctx *sctx = shash_desc_ctx(desc);
return skein_256_init(sctx, SKEIN256_DIGEST_BIT_SIZE);
}
> +
> +int skein256_update(struct shash_desc *desc, const u8 *data,
> + unsigned int len)
> +{
> + struct skein_256_ctx *sctx = shash_desc_ctx(desc);
> + size_t hbl = (size_t) len;
Pointless case. Pointless assignment.
> + int ret = skein_256_update(sctx, data, hbl);
> +
> + return ret;
> +}
This should be:
int skein256_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
struct skein_256_ctx *sctx = shash_desc_ctx(desc);
return skein_256_update(sctx, data, len);
}
> +EXPORT_SYMBOL(skein256_update);
> +
> +/* Add padding and return the message digest. */
> +static int skein256_final(struct shash_desc *desc, u8 *out)
> +{
> + struct skein_256_ctx *sctx = shash_desc_ctx(desc);
> + int ret = skein_256_final(sctx, out);
> + return ret;
> +}
Same.
> +
> +static int skein512_init(struct shash_desc *desc)
> +{
> + struct skein_512_ctx *sctx = shash_desc_ctx(desc);
> +
> + int ret = skein_512_init(sctx, SKEIN512_DIGEST_BIT_SIZE);
> +
> + return ret;
> +}
Same.
> +
> +int skein512_update(struct shash_desc *desc, const u8 *data,
> + unsigned int len)
> +{
> + struct skein_512_ctx *sctx = shash_desc_ctx(desc);
> + size_t hbl = (size_t) len;
> + int ret = skein_512_update(sctx, data, hbl);
> +
> + return ret;
> +}
Same.
> +EXPORT_SYMBOL(skein512_update);
> +
> +/* Add padding and return the message digest. */
> +static int skein512_final(struct shash_desc *desc, u8 *out)
> +{
> + struct skein_512_ctx *sctx = shash_desc_ctx(desc);
> + int ret = skein_512_final(sctx, out);
> + return ret;
> +}
Same.
> +
> +static int skein1024_init(struct shash_desc *desc)
> +{
> + struct skein_1024_ctx *sctx = shash_desc_ctx(desc);
> +
> + int ret = skein_1024_init(sctx, SKEIN1024_DIGEST_BIT_SIZE);
> +
> + return ret;
> +}
Same.
> +
> +int skein1024_update(struct shash_desc *desc, const u8 *data,
> + unsigned int len)
> +{
> + struct skein_1024_ctx *sctx = shash_desc_ctx(desc);
> + size_t hbl = (size_t) len;
> + int ret = skein_1024_update(sctx, data, hbl);
> +
> + return ret;
> +}
Same.
> +EXPORT_SYMBOL(skein1024_update);
> +
> +/* Add padding and return the message digest. */
> +static int skein1024_final(struct shash_desc *desc, u8 *out)
> +{
> + struct skein_1024_ctx *sctx = shash_desc_ctx(desc);
> + int ret = skein_1024_final(sctx, out);
> + return ret;
> +}
Same.
regarsd,
dan carpenter
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@...r.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Please read the FAQ at http://www.tux.org/lkml/
Powered by blists - more mailing lists