[Debian-med-packaging] Bug#667214: Help needed to fix gcc 4.7 bug in jellyfish package

Andreas Tille andreas at an3as.eu
Thu May 3 13:47:42 UTC 2012 

----- Forwarded message from Guillaume Marcais <guillaume.marcais at marcais.net> -----

Date: Thu, 3 May 2012 09:43:00 -0400
From: Guillaume Marcais <guillaume.marcais at marcais.net>
To: Andreas Tille <andreas at an3as.eu>
Subject: Re: Help needed to fix gcc 4.7 bug in jellyfish package
X-Spam_score: -0.7

On Thu, May 3, 2012 at 8:46 AM, Andreas Tille <andreas at an3as.eu> wrote:
> Hi Guillaume,
>
> in the Debian bug tracking system
>
>   http://bugs.debian.org/667214
>
> a build problem when using gcc-4.7 was reported.  Below you can read
> some discussion about a possible fix.
>
> Michael, I can confirm that I also tried to s/uint_t/int/ in
> parse_dna.hpp with the same result (same error message) after I did my
> initial posting (that's why I did not felt a real need to send another
> mail).
>
> And yes, I agree that assigning negative values to unsigned variables
> smells like done with some purpose which might stay hidden from the
> first look and I would have definitely asked upstream about any problem
> my suggested patch might have caused.  I just felt like doing some
> investigation into the problem might make sense.  However, it seems
> that this does not lead to any progres.

Thanks for the bug report, I'll look into it. I have not tried gcc 4.7 yet.

I think the idea was to separate the good codes (i.e. the DNA bases
ACGT) from the error codes. The error codes have the MSB set to 1. At
some point in time I was checking for that condition. It appears that
I don't any more and it looks like a pointless dirty trick. I will
probably remove it.

I'll compile gcc 4.7 on our system and make sure that it Jellyfish
compiles properly.

Guillaume.

> Guillaume, could you enlighten us a bit about this trick?
>
> Kind regards
>
>        Andreas.
>
> On Thu, May 03, 2012 at 09:52:09AM +0200, Michael Wild wrote:
>> On 05/02/2012 08:33 AM, Andreas Tille wrote:
>> > Hi,
>> >
>> > I tried to fix the problem in the jellyfish package but the general
>> > hints given did not helped me really.  Any more precise help to fix
>> > this problem:
>> >
>> > parse_dna.cc:97:3: error: narrowing conversion of '-3' from 'int' to 'const uint_t {aka const long unsigned int}' inside { } is ill-formed in C++11 [-Werror=narrowing]
>> >
>> > My first idea was to do
>> >
>> > --- jellyfish.orig/jellyfish/parse_dna.cc
>> > +++ jellyfish/jellyfish/parse_dna.cc
>> > @@ -57,7 +57,7 @@
>> >      }
>> >    }
>> >
>> > -  const uint_t parse_dna::codes[256] = {
>> > +  const int parse_dna::codes[256] = {
>> >      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -2, -3, -3, -3, -3, -3,
>> >      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3,
>> >      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -1, -3, -3,
>> > --- jellyfish.orig/jellyfish/parse_dna.hpp
>> > +++ jellyfish/jellyfish/parse_dna.hpp
>> > @@ -55,7 +55,7 @@
>> >      static uint64_t mer_string_to_binary(const char *in, uint_t klen) {
>> >        uint64_t res = 0;
>> >        for(uint_t i = 0; i < klen; i++) {
>> > -        const uint_t c = parse_dna::codes[(uint_t)*in++];
>> > +        const int c = parse_dna::codes[(int)*in++];
>> >          if(c & CODE_NOT_DNA)
>> >            return 0;
>> >          res = (res << 2) | c;
>> >
>> >
>> > because it makes no sense to initialise uint with negative numbers but
>> > this did not changed the error message which sounds totally strange to
>> > me.
>> >
>> > Kind regards
>> >
>> >         Andreas.
>> >
>>
>> You missed the declaration of parse_dna::codes in parse_dna.hpp.
>>
>>
>> diff --git a/jellyfish/parse_dna.cc b/jellyfish/parse_dna.cc
>> index ab3ec64..9ea5ae1 100644
>> --- a/jellyfish/parse_dna.cc
>> +++ b/jellyfish/parse_dna.cc
>> @@ -57,7 +57,7 @@ namespace jellyfish {
>>      }
>>    }
>>
>> -  const uint_t parse_dna::codes[256] = {
>> +  const int parse_dna::codes[256] = {
>>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -2, -3, -3, -3, -3, -3,
>>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3,
>>      -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -3, -1, -3, -3,
>> diff --git a/jellyfish/parse_dna.hpp b/jellyfish/parse_dna.hpp
>> index 0435ae2..7ef8afd 100644
>> --- a/jellyfish/parse_dna.hpp
>> +++ b/jellyfish/parse_dna.hpp
>> @@ -46,7 +46,7 @@ namespace jellyfish {
>>       * '\n': map to -2. ignore
>>       * Other ASCII: map to -3. Skip to next line
>>       */
>> -    static const uint_t codes[256];
>> +    static const int codes[256];
>>      static const uint_t CODE_RESET = -1;
>>      static const uint_t CODE_IGNORE = -2;
>>      static const uint_t CODE_COMMENT = -3;
>> @@ -55,7 +55,7 @@ namespace jellyfish {
>>      static uint64_t mer_string_to_binary(const char *in, uint_t klen) {
>>        uint64_t res = 0;
>>        for(uint_t i = 0; i < klen; i++) {
>> -        const uint_t c = parse_dna::codes[(uint_t)*in++];
>> +        const int c = parse_dna::codes[(uint_t)*in++];
>>          if(c & CODE_NOT_DNA)
>>            return 0;
>>          res = (res << 2) | c;
>>
>> That said, assigning -3 to an unsigned int seems to be a pretty
>> conscious choice to me, so it might have been done on purpose to create
>> a wrap-around. Also, the same pattern shows up many other places (e.g.
>> parse_dna::CODE_RESET, parse_dna::CODE_IGNORE, ...). IMHO bad practice,
>> but plausible. Probably it's best to contact upstream about this and ask
>> what their original intention was.
>>
>> Michael
>>
>>
>> --
>> To UNSUBSCRIBE, email to debian-mentors-REQUEST at lists.debian.org
>> with a subject of "unsubscribe". Trouble? Contact listmaster at lists.debian.org
>> Archive: http://lists.debian.org/4FA23929.3000509@gmail.com
>>
>>
>
> --
> http://fam-tille.de


----- End forwarded message -----

-- 
http://fam-tille.de

More information about the Debian-med-packaging mailing list