patch, 3 partsJanitor minor fixes on SHA1

> There's no need to update the pointer and remaining length before
> leaving or calling the SHA1 sub function.
>
> Additionnaly, the partial block code could be looking more like
> the full block handling branch.
>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> ---
>  block-sha1/sha1.c | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
> index a8d4bf9..c1af112 100644
> --- a/block-sha1/sha1.c
> +++ b/block-sha1/sha1.c
> @@ -248,11 +248,11 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
>  			left = len;
>  		memcpy(lenW + (char *)ctx->W, data, left);
>  		lenW = (lenW + left) & 63;
> -		len -= left;
> -		data = ((const char *)data + left);
>  		if (lenW)
>  			return;
>  		blk_SHA1_Block(ctx, ctx->W);
> +		data = ((const char *)data + left);
> +		len -= left;
>  	}
>  	while (len >= 64) {
>  		blk_SHA1_Block(ctx, data);

>
> Signed-off-by: Yann Droneaud <ydroneaud@opteya.com>
> ---
>  block-sha1/sha1.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/block-sha1/sha1.c b/block-sha1/sha1.c
> index c1af112..a7c4470 100644
> --- a/block-sha1/sha1.c
> +++ b/block-sha1/sha1.c
> @@ -246,7 +246,7 @@ void blk_SHA1_Update(blk_SHA_CTX *ctx, const void *data, unsigned long len)
>  		unsigned int left = 64 - lenW;
>  		if (len < left)
>  			left = len;
> -		memcpy(lenW + (char *)ctx->W, data, left);
> +		memcpy((char *)ctx->W + lenW, data, left);
>  		lenW = (lenW + left) & 63;
>  		if (lenW)
>  			return;

> Yann Droneaud <ydroneaud@opteya.com> writes:
> 
> > One memcpy() argument is computed from a pointer added to an integer:
> > eg. int + pointer. It's unusal.
> > Let's write it in the correct order: pointer + offset.
> 
> Meh.
> 
> Both are correct.  Aren't ctx->w[lenW] and lenW[ctx-w] both correct,
> even?
> 

>> Both are correct.  Aren't ctx->w[lenW] and lenW[ctx-w] both correct,
>> even?
>>
>
> "correct" in my commit log message should be read as "the way it's used
> by most C developer".
>
> It's again a cosmetic fix.

> 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;

> 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.
> 

> > > 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.

> > 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.

> > > 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.
> 
> Sorry, you're right, that's a different work array (though it has the
> identical issue, no?).

> > > 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.
> 
> It is simpler in the sense that it does not have any side effects (like
> changing how every user of the data structure needs to index it).
> 

> > > 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.
> 
> So...if we are not ready to run on an ILP system after this change, then
> what is the purpose?
> 

> 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;
>  
>  void blk_SHA1_Init(blk_SHA_CTX *ctx);

> Yann Droneaud <ydroneaud@opteya.com> writes:
> 
> > 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>
> > ---
> 
> As we do not work with 16-bit integers anyway, 16 integers occupy 64
> bytes anyway.
>