Re: [PATCH v2 1/7] crypto: arc4 - refactor arc4 core code into separate library

[Eric Biggers <ebiggers@kernel.org>]

Just some bike shedding:

On Sun, Jun 09, 2019 at 01:55:03PM +0200, Ard Biesheuvel wrote:
> diff --git a/include/crypto/arc4.h b/include/crypto/arc4.h
> index 5b2c24ab0139..62ac95ec6860 100644
> --- a/include/crypto/arc4.h
> +++ b/include/crypto/arc4.h
> @@ -6,8 +6,21 @@
>  #ifndef _CRYPTO_ARC4_H
>  #define _CRYPTO_ARC4_H
>  
> +#include <linux/types.h>
> +
>  #define ARC4_MIN_KEY_SIZE	1
>  #define ARC4_MAX_KEY_SIZE	256
>  #define ARC4_BLOCK_SIZE		1
>  
> +struct crypto_arc4_ctx {
> +	u32 S[256];
> +	u32 x, y;
> +};
> +
> +int crypto_arc4_set_key(struct crypto_arc4_ctx *ctx, const u8 *in_key,
> +			unsigned int key_len);
> +
> +void crypto_arc4_crypt(struct crypto_arc4_ctx *ctx, u8 *out, const u8 *in,
> +		       unsigned int len);

How about naming these just arc4_* instead of crypto_arc4_*?  The crypto_*
prefix is already used mostly for crypto API helper functions, i.e. functions
that take take one of the abstract crypto API types like crypto_skcipher,
shash_desc, etc.  For lib functions, the bare name seems more appropriate.  See
e.g. sha256_update() vs. crypto_sha256_update().

> +++ b/lib/crypto/Makefile
> @@ -0,0 +1,3 @@
> +# SPDX-License-Identifier: GPL-2.0
> +
> +obj-$(CONFIG_CRYPTO_LIB_ARC4) += libarc4.o
> diff --git a/lib/crypto/libarc4.c b/lib/crypto/libarc4.c
> new file mode 100644
> index 000000000000..b828af2cc03b
> --- /dev/null
> +++ b/lib/crypto/libarc4.c
> @@ -0,0 +1,74 @@

How about arc4.c instead of libarc4.c?  The second "lib" is redundant, given
that the file is already in the lib/ directory.

- Eric