Re: Bug#876839: staden-io-lib FTBFS on big endian: error: invalid operands to binary & (original) (raw)

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

Reply to: