git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Jeff King <peff@peff.net>
To: Yann Droneaud <ydroneaud@opteya.com>
Cc: git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>
Subject: Re: [PATCH 3/3] sha1: use char type for temporary work buffer
Date: Wed, 12 Sep 2012 14:38:33 -0400	[thread overview]
Message-ID: <20120912183833.GA20795@sigill.intra.peff.net> (raw)
In-Reply-To: <a8c30a998cad6a7b38bd983e7689a628567a8176.1347442430.git.ydroneaud@opteya.com>

On Wed, Sep 12, 2012 at 12:30:45PM +0200, Yann Droneaud wrote:

> The SHA context is holding a temporary buffer for partial block.
> 
> This block must 64 bytes long. It is currently described as
> an array of 16 integers.
> 
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> ---
>  block-sha1/sha1.h | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/block-sha1/sha1.h b/block-sha1/sha1.h
> index b864df6..d29ff6a 100644
> --- a/block-sha1/sha1.h
> +++ b/block-sha1/sha1.h
> @@ -9,7 +9,7 @@
>  typedef struct {
>  	unsigned long long size;
>  	unsigned int H[5];
> -	unsigned int W[16];
> +	unsigned char W[64];
>  } blk_SHA_CTX;

Wouldn't this break all of the code that is planning to index "W" by
32-bit words (see the definitions of setW in block-sha1/sha1.c)?

You do not describe an actual problem in the commit message, but reading
between the lines it would be "system X would like to use block-sha1,
but has an "unsigned int" that is not 32 bits". IOW, an ILP64 type of
architecture. Do you have some specific platform in mind?

If that is indeed the problem, wouldn't the simplest fix be using
uint32_t instead of "unsigned int"?

Moreover, would that be sufficient to run on such a platform? At the
very least, "H" above would want the same treatment. And I would not be
surprised if some of the actual code in block-sha1/sha1.c needed
updating, as well.

-Peff

  reply	other threads:[~2012-09-12 18:38 UTC|newest]

Thread overview: 14+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-09-12 10:01 [PATCH 0/3] Janitor minor fixes on SHA1 Yann Droneaud
2012-09-12 10:01 ` [PATCH 1/3] sha1: update pointer and remaining length after subfunction call Yann Droneaud
2012-09-12 18:35   ` Junio C Hamano
2012-09-12 10:21 ` [PATCH 2/3] sha1: clean pointer arithmetic Yann Droneaud
2012-09-12 18:37   ` Junio C Hamano
2012-09-12 20:50     ` Yann Droneaud
2012-09-12 21:25       ` Bruce Korb
2012-09-12 10:30 ` [PATCH 3/3] sha1: use char type for temporary work buffer Yann Droneaud
2012-09-12 18:38   ` Jeff King [this message]
2012-09-12 20:37     ` Yann Droneaud
2012-09-12 21:04       ` Jeff King
2012-09-13  6:11         ` Yann Droneaud
2012-09-12 18:42   ` Junio C Hamano
2012-09-12 20:42     ` Yann Droneaud

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20120912183833.GA20795@sigill.intra.peff.net \
    --to=peff@peff.net \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=ydroneaud@opteya.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).