On 2 Feb 2018, at 15:49, David Matthew Mattli <[email protected]> wrote: > > James Clarke <[email protected]> writes: > >> user [email protected] >> usertags sparc64 >> thanks >> >> (You missed the User: pseudoheader) >> >>> On 2 Feb 2018, at 14:46, David Matthew Mattli <[email protected]> wrote: >>> >>> Package: abyss >>> Severity: normal >>> Tags: patch upstream >>> Usertags: sparc64 >>> >>> Dear Maintainer, >>> >>> This package currently FTBFS on sparc64 due to the >>> PairedDBG_LoadAlgorithm test failing with a SIGBUS. The Kmer.load and >>> Kmer.storeReverse methods in the Common/Kmer.cpp file cast a uint8_t* >>> to a size_t* without ensuring the pointer value has the proper >>> alignment. >>> >>> To fix this I added an aligned stack allocated buffer and memcpy to >>> that. Stack allocated is appropriate because the buffer size is >>> small(32 bytes) and known at compile time. >> >> Hi, >> As a sparc64 porter, thanks for fixing packages! Just a couple of >> comments on the patch. >> >>> --- a/Common/Kmer.cpp >>> +++ b/Common/Kmer.cpp >>> @@ -188,9 +188,10 @@ >>> Seq seq; >>> #if MAX_KMER > 96 >>> # if WORDS_BIGENDIAN >>> - const size_t *s = reinterpret_cast<const size_t*>(src); >>> + size_t buf[Kmer::NUM_BYTES]; >> >> Should be divided by sizeof(size_t). Also, why not call it s? That should >> reduce the number of changes. >> >>> + memcpy(buf, src, Kmer::NUM_BYTES); >>> size_t *d = reinterpret_cast<size_t*>(&seq + 1); >>> - copy(s, s + Kmer::NUM_BYTES/sizeof(size_t), >>> reverse_iterator<size_t*>(d)); >>> + copy(buf, buf + Kmer::NUM_BYTES/sizeof(size_t), >>> reverse_iterator<size_t*>(d)); >>> # else >>> uint8_t *d = reinterpret_cast<uint8_t*>(&seq); >>> memcpy(d, src, sizeof seq); >>> @@ -235,9 +236,10 @@ >>> #if MAX_KMER > 96 >>> # if WORDS_BIGENDIAN >>> const size_t *s = reinterpret_cast<const size_t*>(&seq); >>> - size_t *d = reinterpret_cast<size_t*>(dest); >>> + size_t d[Kmer::NUM_BYTES]; >> >> Ditto for the size (this time you used the same name as before). >> >>> copy(s, s + Kmer::NUM_BYTES/sizeof(size_t), >>> reverse_iterator<size_t*>(d + >>> Kmer::NUM_BYTES/sizeof(size_t))); >>> + memcpy(dest, d, Kmer::NUM_BYTES); >>> reverse(dest, dest + Kmer::NUM_BYTES); >>> # else >>> memcpy(dest, &seq, Kmer::NUM_BYTES); >> >> Regards, >> James > > Thanks for the quick feedback! How does this revised patch look?
Looks sensible as a non-maintainer. Let's wait for a debian-med person to review (technically I have commit rights, but I know nothing about this package). James
