On Wed, Sep 27, 2023 at 07:04:50PM +0200, Stefano Brivio wrote: > On Sat, 23 Sep 2023 00:06:26 +1000 > David Gibson 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 > > --- > > 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