| Message ID | 1430242716.660.70.camel@schen9-desk2.jf.intel.com (mailing list archive) |
|---|---|
| State | RFC |
| Headers | show |
>>>>> "Tim" == Tim Chen <tim.c.chen@linux.intel.com> writes:
Tim> There are a lot of duplicated code between crc_t10dif_update and
Tim> crc_t10dif. The only difference is for the update function we
Tim> import the crc value. I will prefer that we consolidate the code
Tim> into a local inline function that crc_t10dif_update and crc_t10dif
Tim> invoke to get rid of all the duplication.
I'm OK with that approach.
2015-04-29 2:38 GMT+09:00 Tim Chen <tim.c.chen@linux.intel.com>: > There are a lot of duplicated code between crc_t10dif_update and > crc_t10dif. The only difference is for the update function > we import the crc value. I will prefer that we consolidate the code > into a local inline function that crc_t10dif_update and > crc_t10dif invoke to get rid of all the duplication. > > Probably something like: Looks good. I'll take this code. > diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c > index dfe6ec1..0248f78 100644 > --- a/lib/crc-t10dif.c > +++ b/lib/crc-t10dif.c > @@ -19,7 +19,7 @@ > static struct crypto_shash *crct10dif_tfm; > static struct static_key crct10dif_fallback __read_mostly; > > -__u16 crc_t10dif(const unsigned char *buffer, size_t len) > +static inline __u16 __crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len, bool update) > { > struct { > struct shash_desc shash; > @@ -28,17 +28,33 @@ __u16 crc_t10dif(const unsigned char *buffer, size_t len) > int err; > > if (static_key_false(&crct10dif_fallback)) > - return crc_t10dif_generic(0, buffer, len); > + return crc_t10dif_generic(crc, buffer, len); > > desc.shash.tfm = crct10dif_tfm; > desc.shash.flags = 0; > - *(__u16 *)desc.ctx = 0; > + > + if (update) { > + err = crypto_shash_import(&desc.shash, &crc); > + BUG_ON(err); > + } else > + *(__u16 *)desc.ctx = 0; > > err = crypto_shash_update(&desc.shash, buffer, len); > BUG_ON(err); > > return *(__u16 *)desc.ctx; > } > + > +__u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) > +{ > + return __crc_t10dif_update(crc, buffer, len, true); > +} > +EXPORT_SYMBOL(crc_t10dif_update); > + > +__u16 crc_t10dif(const unsigned char *buffer, size_t len) > +{ > + return __crc_t10dif_update(0, buffer, len, false); > +} > EXPORT_SYMBOL(crc_t10dif); > > > Thanks. > > Tim > -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On Tue, Apr 28, 2015 at 10:38:36AM -0700, Tim Chen wrote: > > + if (update) { > + err = crypto_shash_import(&desc.shash, &crc); > + BUG_ON(err); You don't even have to make this conditional. Just always do the import since it's just doing a memcpy anyway. Cheers,
On Wed, 2015-04-29 at 08:49 +0800, Herbert Xu wrote: > On Tue, Apr 28, 2015 at 10:38:36AM -0700, Tim Chen wrote: > > > > + if (update) { > > + err = crypto_shash_import(&desc.shash, &crc); > > + BUG_ON(err); > > You don't even have to make this conditional. Just always do > the import since it's just doing a memcpy anyway. > Cool, this will simplify things more :) Tim -- To unsubscribe from this list: send the line "unsubscribe linux-crypto" in the body of a message to majordomo@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
diff --git a/lib/crc-t10dif.c b/lib/crc-t10dif.c index dfe6ec1..0248f78 100644 --- a/lib/crc-t10dif.c +++ b/lib/crc-t10dif.c @@ -19,7 +19,7 @@ static struct crypto_shash *crct10dif_tfm; static struct static_key crct10dif_fallback __read_mostly; -__u16 crc_t10dif(const unsigned char *buffer, size_t len) +static inline __u16 __crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len, bool update) { struct { struct shash_desc shash; @@ -28,17 +28,33 @@ __u16 crc_t10dif(const unsigned char *buffer, size_t len) int err; if (static_key_false(&crct10dif_fallback)) - return crc_t10dif_generic(0, buffer, len); + return crc_t10dif_generic(crc, buffer, len); desc.shash.tfm = crct10dif_tfm; desc.shash.flags = 0; - *(__u16 *)desc.ctx = 0; + + if (update) { + err = crypto_shash_import(&desc.shash, &crc); + BUG_ON(err); + } else + *(__u16 *)desc.ctx = 0; err = crypto_shash_update(&desc.shash, buffer, len); BUG_ON(err); return *(__u16 *)desc.ctx; } + +__u16 crc_t10dif_update(__u16 crc, const unsigned char *buffer, size_t len) +{ + return __crc_t10dif_update(crc, buffer, len, true); +} +EXPORT_SYMBOL(crc_t10dif_update); + +__u16 crc_t10dif(const unsigned char *buffer, size_t len) +{ + return __crc_t10dif_update(0, buffer, len, false); +} EXPORT_SYMBOL(crc_t10dif);