Re: [PATCH] sha512: replace open-coded be64 conversions

[Denis Vlasenko <vda@port.imtp.ilyichevsk.odessa.ua>]

On Tuesday 19 April 2005 17:41, Jesper Juhl wrote:
> On Tue, 19 Apr 2005, Denis Vlasenko wrote:
> 
> > On Tuesday 19 April 2005 12:04, Jesper Juhl wrote:
> > > On Tue, 19 Apr 2005, Denis Vlasenko wrote:
> > > 
> > > > This + next patch were "modprobe tcrypt" tested.
> > > > See next mail.
> > > 
> > > Could you please send patches inline instead of as attachments. 
> > > Attachments mean there's more work involved in getting at them to read 
> > > them, and they are a pain when you want to reply and quote the patch to 
> > > comment on it.
> > > Thanks.
> > 
> > I'm afraid Kmail tend to subtly mangle inlined patches.
> > Then people start to complain that patch does not apply, and rightly so. :(
> > 
> > However, I can try.
> > 
> Thanks.
> 
> A few small comments below.
> 
> 
> [...]
> > @@ -483,12 +483,8 @@ static int anubis_setkey(void *ctx_arg, 
> >  	ctx->R = R = 8 + N;
> >  
> >  	/* * map cipher key to initial key state (mu): */
> > -		for (i = 0, pos = 0; i < N; i++, pos += 4) {
> > -		kappa[i] =
> > -			(in_key[pos    ] << 24) ^
> > -			(in_key[pos + 1] << 16) ^
> > -			(in_key[pos + 2] <<  8) ^
> > -			(in_key[pos + 3]      );
> > +	for (i = 0; i < N; i++) {
> > +		kappa[i] = be32_to_cpu( ((__be32*)in_key)[i] );
>                                          ^^^^^^^            ^
>                  is this cast really nessesary ?      No space there.

Yes, it is necessary. It (a) casts pointer to u32*,
this allows index [i] to work as inteneded.
Adn (b) it marks value as big-endian. IIRC this is needed
for sparse checking.

Spaces are there because otherwise it is a bit hard to visually
check for correct number and placement of three pairs of ()'s.

> >  	}
>         ^
>      Those braces are superfluous.

Yes. Since they were there from the start, I decided to not touch 'em.
--
vda

-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/