[Debian-med-packaging] Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary &

[Andreas Tille]

control: forwarded -1 James Bonfield <jkb at sanger.ac.uk>
control: tags -1 upstream

Hi James,

There is a bug report against the Debian package of the latest
io_lib version.  See the full bug log here:

    https://bugs.debian.org/876839

Below you can read about a suggested solution.

Kind regards

       Andreas.

On Tue, Sep 26, 2017 at 10:31:01PM +0200, Christian Seiler wrote:
> On 09/26/2017 10:06 PM, Andreas Tille wrote:
> >> ...
> >> In file included from bgzip.c:56:0:
> >> bgzip.c: In function 'gzi_index_dump':
> >> ../io_lib/os.h:127:10: error: invalid operands to binary & (have 'uint64_t * {aka long long unsigned int *}' and 'long long int')
> >>      (((x & 0x00000000000000ffLL) << 56) + \
> >>           ^
> >> ../io_lib/os.h:185:20: note: in expansion of macro 'iswap_int8'
> >>  #define le_int8(x) iswap_int8((x))
> >>                     ^~~~~~~~~~
> >> bgzip.c:190:16: note: in expansion of macro 'le_int8'
> >>      if (fwrite(le_int8(&n), sizeof(n), 1, idx_f) != 1)
> >>                 ^~~~~~~
> 
> This code is completely wrong.
> 
> le_int8 appears to do a 64 bit byte order swap to adjust the
> endianness of a quantity. What bgzip.c does at this point is the
> following (removed if() for clarity):
> 
> uint64_t n = idx->n;
> fwrite(le_int8(&n), sizeof(n), 1, idx_f);
> 
> &n is the pointer to the 'n' variable, but you really don't want
> to byte swap a pointer to a local variable before passing it to
> a function that reads that pointer (also note that the pointer
> could be 32 bit on 32 bit systems!).
> 
> What you presumably want to do is byte swap the _contents_ of the
> pointer (especially since n is a 64 bit integer). If you look at
> the read code in the same file this is actually what happens:
> 
>     if (8 != fread(&n, 1, 8, fp))
> 	goto err;
>     n = le_int8(n);
> 
> So what you'd rather want to have is the following code:
> 
> uint64_t n = le_int8(idx->n);
> fwrite(&n, sizeof(n), 1, idx_f);
> 
> Or, if I adjust the entirety of that section in the original write
> code:
> 
>     int i;
>     uint64_t n = le_int8(idx->n);
>     if (fwrite(&n), sizeof(n), 1, idx_f) != 1)
> 	goto fail;
>     for (i=0; i<idx->n; i++) {
>         uint64_t nn;
>         nn = le_int8(idx->c_off[i]);
> 	if (fwrite(&nn, sizeof(nn), 1, idx_f) != 1)
> 	    goto fail;
>         nn = le_int8(idx->u_off[i]);
> 	if (fwrite(&nn, sizeof(nn), 1, idx_f) != 1)
> 	    goto fail;
>     }
> 
> That should fix the compiler error you're seeing.
> 
> The only reason that doesn't fail on Little Endian is because the
> le_int8(x) function is a no-op on those systems and just passes
> through the original pointer.
> 
> Regards,
> Christian
> 

-- 
http://fam-tille.de