On Fri, Apr 21, 2017 at 04:20:24AM +0000, Lumin wrote:
> Updated package was uploaded to mentors:
> https://mentors.debian.net/debian/pool/main/h/highwayhash/highwayhash_0~20170419-g1f4a24f-1.dsc
>
> Changes:
> * fixed the mistaken /lib/<multiarch-triplet>/libxxx.a install path.
> The static library didn't drop
> sip_tree_hash.o object.
> * patched upstream makefile to produce a shared object file
> (sip_tree_hash.o is dropped from so file)
> * added symbols control file
> * override dh_auto_test to run upstream test binaries (except for the
> "benchmark").
>
> Is it acceptable now?
It does look uploadable, yeah, even though there's a bunch of issues. It's
up to you whether you want to get it good first or to upload present state
then improve it incrementally. Please say what you prefer.
The bad news is, it succeeded only on amd64.
On other architectures, the closest it came on x32 (a non-release arch) --
builds ok, fails only at dpkg-gensymbols due to symbol mismatches.
One warning: left shift count >= width of type [-Wshift-count-overflow]
sounds like it might be broken, though[1].
On arm64 it fails with:
g++: error: unrecognized command line option '-mavx2'
On armhf there's both the shift width issue, and:
error: #error "Port"
On i386, all of the above, plus:
error: '_mm_cvtsi64_si128' was not declared in this scope
I assume you neither have access to porterboxes nor an array of hardware at
home (and qemu can be tricky), so it might be easiest for you to abuse the
buildds or someone who can test.
Other issues:
Please add:
Multi-Arch: same
to libhighwayhash0's section in the control file. You install stuff to the
proper paths so there's no reason for it to be not coinstallable.
Some simple docs would be nice -- RTFSing the headers is not pretty.
This could work:
.--====[ highwayhash.3 ]
.TH highwayhash 3
.SH NAME
highwayhash \- fast strong 64-bit hash functions
.SH SYNOPSIS
.B #include <highwayhash/c_bindings.h>
uint64_t SipHashC(const uint64_t* key, const char* bytes, const uint64_t size);
uint64_t SipHash13C(const uint64_t* key, const char* bytes, const uint64_t size);
uint64_t HighwayHash64(const HHKey key, const char* bytes, const uint64_t size);
.SH DESCRIPTION
blah blah blah, SipHash wants an uint64_t[2] key while HighwayHash
uint64_t[4]
`----
(with blah blah replaced with the prose)
The C interface looks more comfortable to use even from C++, so I'd describe
it first (or even exclusively for now).
Manually mucking with optimization variant selection provides only a very
minor speedup, so I wouldn't bother describing those. Especially that with
gcc-6 per-ISA variants can be bound at library load time -- if the library
uses that functionality it could eliminate the overhead altogether. That's
a matter for the upstream, though.
Lemme share a sanity test I used:
.--====
#include <stdio.h>
#include <inttypes.h>
#include <highwayhash/c_bindings.h>
static const uint64_t shkey[2]={0xdeadbeef,0xcafebabe};
static const HHKey hhkey={3,14,15,926}; // 4×uint64_t
int main()
{
printf("%016"PRIx64"\n", SipHashC(shkey, "meow", 4));
printf("%016"PRIx64"\n", SipHash13C(shkey, "meow", 4));
printf("%016"PRIx64"\n", HighwayHash64(hhkey, "meow", 4));
return 0;
}
---- produces:
af0a25067c014659
600708416bfbe7ad
01aeb7e482f04c46
`----
[1]. Such shifts produce only a warning, but are notorious for giving wrong
results:
int lines = 32;
u32 mask = (1 << lines) - 1; // 00000000 on x86
u32 mask = (1 << lines) - 1; // ffffffff on arm (32)
u32 mask = (1 << lines) - 1; // 00000000 on arm64
u32 mask = (1ULL << lines) - 1; // ffffffff everywhere
--
⢀⣴⠾⠻⢶⣦⠀ Meow!
⣾⠁⢠⠒⠀⣿⡁
⢿⡄⠘⠷⠚⠋⠀ Collisions shmolisions, let's see them find a collision or second
⠈⠳⣄⠀⠀⠀⠀ preimage for double rot13!