Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-28 Thread Andreas Tille
Hi James,

On Thu, Sep 28, 2017 at 09:52:33AM +0100, James Bonfield wrote:
> > > 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(), sizeof(n), 1, idx_f);
> 
> Now fixed and pushed.  Thanks.

Thanks for the very quick help.  Unfortunately sourceforge seems to be
unavailable (same as for other projects[1]).  Your wording "pushed"
implies that you are familiarising with Git - so my hint I gave in my
response[1] might apply even more.

I'd be interested to fix this so could you find some other means (some
temporary web space or even attaching to private e-mail) to provide me
with the new source tarball?

Kind regards

  Andreas.

[1] https://lists.debian.org/debian-med/2017/09/msg00093.html 

-- 
http://fam-tille.de



Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-28 Thread James Bonfield
Hello Andreas,

On Tue, Sep 26, 2017 at 10:46:15PM +0200, Andreas Tille wrote:
> 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

Thanks for the link.

Christian is quite correct in his reply.  I don't know what I was
thinking when I wrote fwrite(le_int8()) as clearly we need the value
and not the pointer.  I was probably thinking fwrite needs the pointer
and just got it wrong in my head.  This is what I get for not having
an automatic big-endian continuous integration system I guess.

I'll correct it.

James
-- 
James Bonfield (j...@sanger.ac.uk) | Hora aderat briligi. Nunc et Slythia Tova
  | Plurima gyrabant gymbolitare vabo;
  A Staden Package developer: | Et Borogovorum mimzebant undique formae,
https://sf.net/projects/staden/   | Momiferique omnes exgrabure Rathi. 


-- 
 The Wellcome Trust Sanger Institute is operated by Genome Research 
 Limited, a charity registered in England with number 1021457 and a 
 company registered in England with number 2742969, whose registered 
 office is 215 Euston Road, London, NW1 2BE. 



Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-28 Thread James Bonfield
> > 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(), sizeof(n), 1, idx_f);

Now fixed and pushed.  Thanks.

James

-- 
James Bonfield (j...@sanger.ac.uk) | Hora aderat briligi. Nunc et Slythia Tova
  | Plurima gyrabant gymbolitare vabo;
  A Staden Package developer: | Et Borogovorum mimzebant undique formae,
https://sf.net/projects/staden/   | Momiferique omnes exgrabure Rathi. 


-- 
 The Wellcome Trust Sanger Institute is operated by Genome Research 
 Limited, a charity registered in England with number 1021457 and a 
 company registered in England with number 2742969, whose registered 
 office is 215 Euston Road, London, NW1 2BE. 



Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-26 Thread Andreas Tille
control: forwarded -1 James Bonfield 
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 & 0x00ffLL) << 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(), 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(), sizeof(n), 1, idx_f);
> 
>  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(, 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(, 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(), sizeof(n), 1, idx_f) != 1)
>   goto fail;
> for (i=0; in; i++) {
> uint64_t nn;
> nn = le_int8(idx->c_off[i]);
>   if (fwrite(, sizeof(nn), 1, idx_f) != 1)
>   goto fail;
> nn = le_int8(idx->u_off[i]);
>   if (fwrite(, 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



Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-26 Thread Christian Seiler
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 & 0x00ffLL) << 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(), 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(), sizeof(n), 1, idx_f);

 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(, 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(, 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(), sizeof(n), 1, idx_f) != 1)
goto fail;
for (i=0; in; i++) {
uint64_t nn;
nn = le_int8(idx->c_off[i]);
if (fwrite(, sizeof(nn), 1, idx_f) != 1)
goto fail;
nn = le_int8(idx->u_off[i]);
if (fwrite(, 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



Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-26 Thread Andreas Tille
control: tag -1 help

Hi,

it seems this problem happens in bgzip interface.  Is there any known
change to the compression library that is incompatible since
staden-iu-lib was successfully uploaded and built (at least on several
architectures) yesterday.

Any help is welcome

 Andreas.

On Tue, Sep 26, 2017 at 12:27:27PM +0300, Adrian Bunk wrote:
> Source: staden-io-lib
> Version: 1.14.9-2
> Severity: serious
> 
> https://buildd.debian.org/status/package.php?p=staden-io-lib=sid
> 
> ...
> 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 & 0x00ffLL) << 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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:128:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0xff00LL) << 40) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:129:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0x00ffLL) << 24) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:130:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0xff00LL) <<  8) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:131:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0x00ffLL) >>  8) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:132:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0xff00LL) >> 24) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:133:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 0x00ffLL) >> 40) + \
>   ^
> ../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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:134:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long unsigned int')
>   ((x & 0xff00LL) >> 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(), sizeof(n), 1, idx_f) != 1)
> ^~~
> ../io_lib/os.h:127:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>  (((x & 0x00ffLL) << 56) + \
>   ^
> ../io_lib/os.h:185:20: note: in expansion of macro 'iswap_int8'
>  #define le_int8(x) iswap_int8((x))
> ^~
> bgzip.c:193:13: note: in expansion of macro 'le_int8'
>   if (fwrite(le_int8(>c_off[i]), sizeof idx->c_off[i], 1, idx_f) != 1)
>  ^~~
> ../io_lib/os.h:128:10: error: invalid operands to binary & (have 'uint64_t * 
> {aka long long unsigned int *}' and 'long long int')
>   ((x & 

Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary

2017-09-26 Thread Adrian Bunk
Source: staden-io-lib
Version: 1.14.9-2
Severity: serious

https://buildd.debian.org/status/package.php?p=staden-io-lib=sid

...
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 & 0x00ffLL) << 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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:128:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0xff00LL) << 40) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:129:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0x00ffLL) << 24) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:130:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0xff00LL) <<  8) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:131:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0x00ffLL) >>  8) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:132:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0xff00LL) >> 24) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:133:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0x00ffLL) >> 40) + \
  ^
../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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:134:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long unsigned int')
  ((x & 0xff00LL) >> 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(), sizeof(n), 1, idx_f) != 1)
^~~
../io_lib/os.h:127:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
 (((x & 0x00ffLL) << 56) + \
  ^
../io_lib/os.h:185:20: note: in expansion of macro 'iswap_int8'
 #define le_int8(x) iswap_int8((x))
^~
bgzip.c:193:13: note: in expansion of macro 'le_int8'
  if (fwrite(le_int8(>c_off[i]), sizeof idx->c_off[i], 1, idx_f) != 1)
 ^~~
../io_lib/os.h:128:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0xff00LL) << 40) + \
  ^
../io_lib/os.h:185:20: note: in expansion of macro 'iswap_int8'
 #define le_int8(x) iswap_int8((x))
^~
bgzip.c:193:13: note: in expansion of macro 'le_int8'
  if (fwrite(le_int8(>c_off[i]), sizeof idx->c_off[i], 1, idx_f) != 1)
 ^~~
../io_lib/os.h:129:10: error: invalid operands to binary & (have 'uint64_t * 
{aka long long unsigned int *}' and 'long long int')
  ((x & 0x00ffLL) << 24) + \
  ^
../io_lib/os.h:185:20: note: in expansion of macro