James Clarke <jrt...@debian.org> writes: > user debian-sparc@lists.debian.org > usertags sparc64 > thanks > > (You missed the User: pseudoheader) > >> On 2 Feb 2018, at 14:46, David Matthew Mattli <d...@mattli.us> 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? Thanks, David --- a/Common/Kmer.cpp +++ b/Common/Kmer.cpp @@ -188,7 +188,8 @@ Seq seq; #if MAX_KMER > 96 # if WORDS_BIGENDIAN - const size_t *s = reinterpret_cast<const size_t*>(src); + size_t s[Kmer::NUM_BYTES/sizeof(size_t)]; + memcpy(s, 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)); # else @@ -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/sizeof(size_t)]; 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);