Re: [PATCH v2 1/2] crypto, sha1: export sha1_update for reuse
On Sat, Jul 30, 2011 at 3:46 PM, Herbert Xu wrote: > On Thu, Jul 28, 2011 at 05:29:35PM +0200, Mathias Krause wrote: >> On Thu, Jul 28, 2011 at 4:58 PM, Herbert Xu >> wrote: >> > On Sun, Jul 24, 2011 at 07:53:13PM +0200, Mathias Krause wrote: >> >> >> >> diff --git a/include/crypto/sha.h b/include/crypto/sha.h >> >> index 069e85b..7c46d0c 100644 >> >> --- a/include/crypto/sha.h >> >> +++ b/include/crypto/sha.h >> >> @@ -82,4 +82,9 @@ struct sha512_state { >> >> u8 buf[SHA512_BLOCK_SIZE]; >> >> }; >> >> >> >> +#if defined(CONFIG_CRYPTO_SHA1) || defined (CONFIG_CRYPTO_SHA1_MODULE) >> >> +extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data, >> >> + unsigned int len); >> >> +#endif >> > >> > Please remove the unnecessary #if. >> >> The function will only be available when crypto/sha1_generic.o will >> either be build into the kernel or build as a module. Without the #if >> a potential wrong user of this function might not be detected as early >> as at compilation time but as late as at link time, or even worse, as >> late as on module load time -- which is pretty bad. IMHO it's better >> to detect errors early, as in reading "error: implicit declaration of >> function ‘crypto_sha1_update’" when trying to compile the code in >> question instead of "Unknown symbol crypto_sha1_update" in dmesg when >> trying to load the module. That for I would like to keep the #if. > > In order to be consistent please remove the ifdef. In most > similar cases in the crypto subsystem we don't do this. As > adding such ifdefs all over the place would gain very little, > I'd much rather you left it out. Noting that this function wasn't exported before and the only user (sha-ssse3) ensures its availability by other means, it should be okay to remove the #if. I'll update the patch accordingly. Any objections to the second patch? Thanks, Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] crypto, sha1: export sha1_update for reuse
On Thu, Jul 28, 2011 at 05:29:35PM +0200, Mathias Krause wrote: > On Thu, Jul 28, 2011 at 4:58 PM, Herbert Xu > wrote: > > On Sun, Jul 24, 2011 at 07:53:13PM +0200, Mathias Krause wrote: > >> > >> diff --git a/include/crypto/sha.h b/include/crypto/sha.h > >> index 069e85b..7c46d0c 100644 > >> --- a/include/crypto/sha.h > >> +++ b/include/crypto/sha.h > >> @@ -82,4 +82,9 @@ struct sha512_state { > >> u8 buf[SHA512_BLOCK_SIZE]; > >> }; > >> > >> +#if defined(CONFIG_CRYPTO_SHA1) || defined (CONFIG_CRYPTO_SHA1_MODULE) > >> +extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data, > >> + unsigned int len); > >> +#endif > > > > Please remove the unnecessary #if. > > The function will only be available when crypto/sha1_generic.o will > either be build into the kernel or build as a module. Without the #if > a potential wrong user of this function might not be detected as early > as at compilation time but as late as at link time, or even worse, as > late as on module load time -- which is pretty bad. IMHO it's better > to detect errors early, as in reading "error: implicit declaration of > function ‘crypto_sha1_update’" when trying to compile the code in > question instead of "Unknown symbol crypto_sha1_update" in dmesg when > trying to load the module. That for I would like to keep the #if. In order to be consistent please remove the ifdef. In most similar cases in the crypto subsystem we don't do this. As adding such ifdefs all over the place would gain very little, I'd much rather you left it out. The one case where this would make sense is if it were a trivial inline in the !defined case. Thanks! -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] crypto, sha1: export sha1_update for reuse
On Thu, Jul 28, 2011 at 4:58 PM, Herbert Xu wrote: > On Sun, Jul 24, 2011 at 07:53:13PM +0200, Mathias Krause wrote: >> >> diff --git a/include/crypto/sha.h b/include/crypto/sha.h >> index 069e85b..7c46d0c 100644 >> --- a/include/crypto/sha.h >> +++ b/include/crypto/sha.h >> @@ -82,4 +82,9 @@ struct sha512_state { >> u8 buf[SHA512_BLOCK_SIZE]; >> }; >> >> +#if defined(CONFIG_CRYPTO_SHA1) || defined (CONFIG_CRYPTO_SHA1_MODULE) >> +extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data, >> + unsigned int len); >> +#endif > > Please remove the unnecessary #if. The function will only be available when crypto/sha1_generic.o will either be build into the kernel or build as a module. Without the #if a potential wrong user of this function might not be detected as early as at compilation time but as late as at link time, or even worse, as late as on module load time -- which is pretty bad. IMHO it's better to detect errors early, as in reading "error: implicit declaration of function ‘crypto_sha1_update’" when trying to compile the code in question instead of "Unknown symbol crypto_sha1_update" in dmesg when trying to load the module. That for I would like to keep the #if. Thanks for the review! Mathias -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v2 1/2] crypto, sha1: export sha1_update for reuse
On Sun, Jul 24, 2011 at 07:53:13PM +0200, Mathias Krause wrote: > > diff --git a/include/crypto/sha.h b/include/crypto/sha.h > index 069e85b..7c46d0c 100644 > --- a/include/crypto/sha.h > +++ b/include/crypto/sha.h > @@ -82,4 +82,9 @@ struct sha512_state { > u8 buf[SHA512_BLOCK_SIZE]; > }; > > +#if defined(CONFIG_CRYPTO_SHA1) || defined (CONFIG_CRYPTO_SHA1_MODULE) > +extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data, > + unsigned int len); > +#endif Please remove the unnecessary #if. Thanks, -- Email: Herbert Xu Home Page: http://gondor.apana.org.au/~herbert/ PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PATCH v2 1/2] crypto, sha1: export sha1_update for reuse
Export the update function as crypto_sha1_update() to not have the need to reimplement the same algorithm for each SHA-1 implementation. This way the generic SHA-1 implementation can be used as fallback for other implementations that fail to run under certain circumstances, like the need for an FPU context while executing in IRQ context. Signed-off-by: Mathias Krause --- crypto/sha1_generic.c |9 + include/crypto/sha.h |5 + 2 files changed, 10 insertions(+), 4 deletions(-) diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c index 0416091..0b6d907 100644 --- a/crypto/sha1_generic.c +++ b/crypto/sha1_generic.c @@ -36,7 +36,7 @@ static int sha1_init(struct shash_desc *desc) return 0; } -static int sha1_update(struct shash_desc *desc, const u8 *data, +int crypto_sha1_update(struct shash_desc *desc, const u8 *data, unsigned int len) { struct sha1_state *sctx = shash_desc_ctx(desc); @@ -70,6 +70,7 @@ static int sha1_update(struct shash_desc *desc, const u8 *data, return 0; } +EXPORT_SYMBOL(crypto_sha1_update); /* Add padding and return the message digest. */ @@ -86,10 +87,10 @@ static int sha1_final(struct shash_desc *desc, u8 *out) /* Pad out to 56 mod 64 */ index = sctx->count & 0x3f; padlen = (index < 56) ? (56 - index) : ((64+56) - index); - sha1_update(desc, padding, padlen); + crypto_sha1_update(desc, padding, padlen); /* Append length */ - sha1_update(desc, (const u8 *)&bits, sizeof(bits)); + crypto_sha1_update(desc, (const u8 *)&bits, sizeof(bits)); /* Store state in digest */ for (i = 0; i < 5; i++) @@ -120,7 +121,7 @@ static int sha1_import(struct shash_desc *desc, const void *in) static struct shash_alg alg = { .digestsize = SHA1_DIGEST_SIZE, .init = sha1_init, - .update = sha1_update, + .update = crypto_sha1_update, .final = sha1_final, .export = sha1_export, .import = sha1_import, diff --git a/include/crypto/sha.h b/include/crypto/sha.h index 069e85b..7c46d0c 100644 --- a/include/crypto/sha.h +++ b/include/crypto/sha.h @@ -82,4 +82,9 @@ struct sha512_state { u8 buf[SHA512_BLOCK_SIZE]; }; +#if defined(CONFIG_CRYPTO_SHA1) || defined (CONFIG_CRYPTO_SHA1_MODULE) +extern int crypto_sha1_update(struct shash_desc *desc, const u8 *data, + unsigned int len); +#endif + #endif -- 1.5.6.5 -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html