On Sun, Jun 9, 2019 at 11:21 PM SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
>
> On Sun, Jun 09, 2019 at 10:24:55PM +0200, Christian Couder wrote:
> > On Sun, Jun 9, 2019 at 11:23 AM SZEDER Gábor <szeder.dev@xxxxxxxxx> wrote:
> > >
> > > New Perl dependencies always make Dscho sad... :)
> >
> > Yeah, I was not sure how to do it properly in shell so I was hoping I
> > would get suggestions about this. Thanks for looking at this!
> >
> > I could have hardcoded the values as it is done in t0011-hashmap.sh,
> > but I thought it was better to find a function that does he job.
>
> Well, I'm fine with hardcoding the expected hash values (in network
> byte order) as well, because then we won't add another git process
> upstream of a pipe that would pop up during audit later...
Ok, I think I will do that then.
> > > So, 'test oidmap' from the previous patch prints the value we want to
> > > check with:
> > >
> > > printf("%u\n", sha1hash(oid.hash));
> >
> > Yeah, I did it this way because "test-hashmap.c" does the same kind of
> > thing to print hashes:
> >
> > printf("%u %u %u %u\n",
> > strhash(p1), memhash(p1, strlen(p1)),
> > strihash(p1), memihash(p1, strlen(p1)));
> >
> > > First, since object ids inherently make more sense as hex values, it
> > > would be more appropriate to print that hash with the '%x' format
> > > specifier,
> >
> > I would be ok with that, but then I think it would make sense to also
> > print hex values in "test-hashmap.c".
> >
> > > and then we wouldn't need Perl's hex() anymore, and thus
> > > could swap the order of the first four bytes in oidmap's hash without
> > > relying on Perl, e.g. with:
> > >
> > > sed -e 's/^\(..\)\(..\)\(..\)\(..\).*/\4\3\2\1/'
> > >
> > > Second, and more importantly, the need for swapping the byte order
> > > indicates that this test would fail on big-endian systems, I'm afraid.
> > > So I think we need an additional bswap32() on the printing side,
> >
> > Ok, but then shouldn't we also use bswap32() in "test-hashmap.c"?
>
> No. The two test scripts/helpers work with different hashes. t0011
> and 'test-hashmap.c' uses the various FNV-1-based hash functions
> (strhash(), memhash(), ...) to calculate an unsigned int hash of the
> items stored in the hashmap, therefore their hashes will be the same
> regardless of endianness.
I see. Thanks for explaining that.
> In an oidmap, however, the hash is simply
> the first four bytes of the object id as an unsigned int as is,
Yeah, I had realized that.
Thanks,
Christian.
