Re: [PATCH 06/10] siphash: Use more hygienic state initialiser

[David Gibson <david@gibson.dropbear.id.au>]

[-- Attachment #1: Type: text/plain, Size: 2923 bytes --]

On Wed, Sep 27, 2023 at 07:04:50PM +0200, Stefano Brivio wrote:
> On Sat, 23 Sep 2023 00:06:26 +1000
> David Gibson <david@gibson.dropbear.id.au> wrote:
> 
> > The PREAMBLE macro sets up the SipHash initial internal state.  It also
> > defines that state as a variable, which isn't macro hygeinic.  With
> > previous changes simplifying this premable, it's now possible to replace it
> > with a macro which simply expands to a C initialisedrfor that state.
> > 
> > Signed-off-by: David Gibson <david@gibson.dropbear.id.au>
> > ---
> >  siphash.c | 29 ++++++++++++-----------------
> >  1 file changed, 12 insertions(+), 17 deletions(-)
> > 
> > diff --git a/siphash.c b/siphash.c
> > index 6932da2..21c560d 100644
> > --- a/siphash.c
> > +++ b/siphash.c
> > @@ -58,15 +58,12 @@
> >  
> >  #define ROTL(x, b) (uint64_t)(((x) << (b)) | ((x) >> (64 - (b))))
> >  
> > -#define PREAMBLE							  \
> > -	uint64_t v[4] = { 0x736f6d6570736575ULL, 0x646f72616e646f6dULL,	  \
> > -			  0x6c7967656e657261ULL, 0x7465646279746573ULL }; \
> > -	int __i;							  \
> > -									  \
> > -	do {								  \
> > -		for (__i = sizeof(v) / sizeof(v[0]) - 1; __i >= 0; __i--) \
> > -			v[__i] ^= k[__i % 2];				  \
> > -	} while (0)
> > +#define SIPHASH_INIT(k) {						\
> > +		0x736f6d6570736575ULL ^ (k)[0],				\
> > +		0x646f72616e646f6dULL ^ (k)[1],				\
> > +		0x6c7967656e657261ULL ^ (k)[0],				\
> > +		0x7465646279746573ULL ^ (k)[1]				\
> 
> I don't think it actually matters (given the rationale for the choice
> of these constants given in the paper), but earlier this was equivalent
> to:
> 
>   0x736f6d6570736575ULL ^ (k)[1],
>   0x646f72616e646f6dULL ^ (k)[0],
>   0x6c7967656e657261ULL ^ (k)[1],
>   0x7465646279746573ULL ^ (k)[0]

No... I don't think it was.  We had:
	for (__i = sizeof(v) / sizeof(v[0]) - 1; __i >= 0; __i--)                                                
		v[__i] ^= k[__i % 2];

So in the first iteration __i == 3, so we get v[3] ^= k[1], and v[3]
is 0x7465646279746573.

> and it matched both reference implementations linked in the file
> header.

Again, I don't think that's correct.  In
https://github.com/veorq/SipHash.git we have:
	v3 ^= k1;
	v2 ^= k0;
	v1 ^= k1;
	v0 ^= k0;

In both cases the order of operations is reversed, but since they're
independent that doesn't matter.  But the point is that the reference
implementation has v0 <-> k0 and v3 <-> k1, rather than the other way
around.

> Anyway, the paper says:
> 
>   ...where k0 and k1 are the little-endian 64-bit words encoding the key
>   k.
> 
> without giving an order, so I guess either interpretation is fine.

Right, I also don't think it actually matters.

-- 
David Gibson			| I'll have my music baroque, and my code
david AT gibson.dropbear.id.au	| minimalist, thank you.  NOT _the_ _other_
				| _way_ _around_!
http://www.ozlabs.org/~dgibson

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]