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!