Re: [PATCH 3/3] sha1: use char type for temporary work buffer
- From
Yann Droneaud <ydroneaud@opteya.com>
- Date
- Sep 12, 2012, 20:37 UTC
- Message-ID
- <1347482230.1961.4.camel@test.quest-ce.net>
- In-Reply-To
- <20120912183833.GA20795@sigill.intra.peff.net>
Le mercredi 12 septembre 2012 à 14:38 -0400, Jeff King a écrit :
Show 27 quoted lines
> 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)?
> That's not the same "W" ... This part of the code is indeed unclear.
Show 8 quoted lines
> 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"? >
It's another way to fix this oddity, but not simpler.
Show 5 quoted lines
> 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. >
ctx->H is actually used as an array of integer, so it would benefits of being declared uint32_t for an ILP64 system. This fix would also be required for blk_SHA1_Block() function.
Regards.
-- Yann Droneaud OPTEYA