Plus some other minor cleanup.
Signed-off-by: Mandeep Singh Baines <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: [email protected]
---
crypto/sha1_generic.c | 39 +++++++++++++++++++--------------------
1 files changed, 19 insertions(+), 20 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..4bdd228 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -40,33 +40,32 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
- unsigned int partial, done;
- const u8 *src;
+ unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
+ u32 temp[SHA_WORKSPACE_WORDS];
+ const u8 *src = data;
- partial = sctx->count & 0x3f;
sctx->count += len;
- done = 0;
- src = data;
- if ((partial + len) > 63) {
- u32 temp[SHA_WORKSPACE_WORDS];
+ if (partial && ((partial + len) >= SHA1_BLOCK_SIZE)) {
+ unsigned int done = SHA1_BLOCK_SIZE - partial;
- if (partial) {
- done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
- src = sctx->buffer;
- }
+ memcpy(sctx->buffer + partial, src, done);
+ sha_transform(sctx->state, sctx->buffer, temp);
+ len -= done;
+ src += done;
+ partial = 0;
+ }
- do {
- sha_transform(sctx->state, src, temp);
- done += 64;
- src = data + done;
- } while (done + 63 < len);
+ while (len >= SHA1_BLOCK_SIZE) {
+ sha_transform(sctx->state, src, temp);
+ len -= SHA1_BLOCK_SIZE;
+ src += SHA1_BLOCK_SIZE;
+ }
+ if (src != data)
memset(temp, 0, sizeof(temp));
- partial = 0;
- }
- memcpy(sctx->buffer + partial, src, len - done);
+
+ memcpy(sctx->buffer + partial, src, len);
return 0;
}
--
1.7.3.1
From: Mandeep Singh Baines <[email protected]>
Date: Wed, 25 May 2011 20:11:17 -0700
> Plus some other minor cleanup.
>
> Signed-off-by: Mandeep Singh Baines <[email protected]>
The temp[] buffer is explicitly places inside the inner most
basic block so that the compiler doesn't allocate the stack
space unless that code path is taken.
Besides the use of the SHA_* macros, I think your changes
actually make the code worse.
On Wed, 2011-05-25 at 23:34 -0400, David Miller wrote:
> From: Mandeep Singh Baines <[email protected]>
> Date: Wed, 25 May 2011 20:11:17 -0700
> > Plus some other minor cleanup.
> > Signed-off-by: Mandeep Singh Baines <[email protected]>
> The temp[] buffer is explicitly places inside the inner most
> basic block so that the compiler doesn't allocate the stack
> space unless that code path is taken.
Does any version of gcc manage to do that?
Regardless, it's still a good idea to keep
declarations in the use scope.
David Miller ([email protected]) wrote:
>
> The temp[] buffer is explicitly places inside the inner most
> basic block so that the compiler doesn't allocate the stack
> space unless that code path is taken.
>
Fixed in V2 (this patch). Thanks for the review.
-- >8 -- (snip)
Plus some other minor cleanup.
Signed-off-by: Mandeep Singh Baines <[email protected]>
Cc: Herbert Xu <[email protected]>
Cc: David S. Miller <[email protected]>
Cc: Joe Perches <[email protected]>
Cc: [email protected]
---
crypto/sha1_generic.c | 33 +++++++++++++++------------------
1 files changed, 15 insertions(+), 18 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..0b56719 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -40,33 +40,30 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int len)
{
struct sha1_state *sctx = shash_desc_ctx(desc);
- unsigned int partial, done;
- const u8 *src;
+ unsigned int partial = sctx->count % SHA1_BLOCK_SIZE;
- partial = sctx->count & 0x3f;
sctx->count += len;
- done = 0;
- src = data;
- if ((partial + len) > 63) {
+ if ((partial + len) >= SHA1_BLOCK_SIZE) {
u32 temp[SHA_WORKSPACE_WORDS];
if (partial) {
- done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
- src = sctx->buffer;
- }
-
- do {
- sha_transform(sctx->state, src, temp);
- done += 64;
- src = data + done;
- } while (done + 63 < len);
+ unsigned int done = SHA1_BLOCK_SIZE - partial;
+ memcpy(sctx->buffer + partial, data, done);
+ sha_transform(sctx->state, sctx->buffer, temp);
+ len -= done;
+ data += done;
+ partial = 0;
+ }
+ while (len >= SHA1_BLOCK_SIZE) {
+ sha_transform(sctx->state, data, temp);
+ len -= SHA1_BLOCK_SIZE;
+ data += SHA1_BLOCK_SIZE;
+ }
memset(temp, 0, sizeof(temp));
- partial = 0;
}
- memcpy(sctx->buffer + partial, src, len - done);
+ memcpy(sctx->buffer + partial, data, len);
return 0;
}
--
1.7.3.1
On Thu, May 26, 2011 at 04:20:58PM -0700, Mandeep Singh Baines wrote:
> David Miller ([email protected]) wrote:
> >
> > The temp[] buffer is explicitly places inside the inner most
> > basic block so that the compiler doesn't allocate the stack
> > space unless that code path is taken.
> >
>
> Fixed in V2 (this patch). Thanks for the review.
>
> -- >8 -- (snip)
>
> Plus some other minor cleanup.
I don't really like the cleanups. In any case, mixing up the
use of SHA1_BLOCK_SIZE with cleanups increases the chance of
introducing a bug.
So please redo the patch with only the addition of SHA1_BLOCK_SIZE
and nothing else.
Thanks,
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt
Modify sha1_update to use SHA1_BLOCK_SIZE.
Signed-off-by: Mandeep Singh Baines <[email protected]>
---
crypto/sha1_generic.c | 11 ++++++-----
1 files changed, 6 insertions(+), 5 deletions(-)
diff --git a/crypto/sha1_generic.c b/crypto/sha1_generic.c
index 0416091..00ae60e 100644
--- a/crypto/sha1_generic.c
+++ b/crypto/sha1_generic.c
@@ -43,25 +43,26 @@ static int sha1_update(struct shash_desc *desc, const u8 *data,
unsigned int partial, done;
const u8 *src;
- partial = sctx->count & 0x3f;
+ partial = sctx->count % SHA1_BLOCK_SIZE;
sctx->count += len;
done = 0;
src = data;
- if ((partial + len) > 63) {
+ if ((partial + len) >= SHA1_BLOCK_SIZE) {
u32 temp[SHA_WORKSPACE_WORDS];
if (partial) {
done = -partial;
- memcpy(sctx->buffer + partial, data, done + 64);
+ memcpy(sctx->buffer + partial, data,
+ done + SHA1_BLOCK_SIZE);
src = sctx->buffer;
}
do {
sha_transform(sctx->state, src, temp);
- done += 64;
+ done += SHA1_BLOCK_SIZE;
src = data + done;
- } while (done + 63 < len);
+ } while (done + SHA1_BLOCK_SIZE <= len);
memset(temp, 0, sizeof(temp));
partial = 0;
--
1.7.3.1
On Thu, Jun 23, 2011 at 12:02:27PM -0700, Mandeep Singh Baines wrote:
> Modify sha1_update to use SHA1_BLOCK_SIZE.
>
> Signed-off-by: Mandeep Singh Baines <[email protected]>
Patch applied. Thanks a lot!
--
Email: Herbert Xu <[email protected]>
Home Page: http://gondor.apana.org.au/~herbert/
PGP Key: http://gondor.apana.org.au/~herbert/pubkey.txt