Re: [PATCH v7 32/32] iotests: Add tests for qcow2 images with extended L2 entries

From: Alberto Garcia <berto@igalia.com>
To: Eric Blake <eblake@redhat.com>, qemu-devel@nongnu.org
Cc: Kevin Wolf <kwolf@redhat.com>, Derek Su <dereksu@qnap.com>,
	Vladimir Sementsov-Ogievskiy <vsementsov@virtuozzo.com>,
	qemu-block@nongnu.org, Max Reitz <mreitz@redhat.com>
Subject: Re: [PATCH v7 32/32] iotests: Add tests for qcow2 images with extended L2 entries
Date: Fri, 29 May 2020 17:07:05 +0200	[thread overview]
Message-ID: <w51v9kevkyu.fsf@maestria.local.igalia.com> (raw)
In-Reply-To: <c2bb44f3-bb7e-7bb0-dbbf-ef1d3273802d@redhat.com>

On Wed 27 May 2020 08:30:06 PM CEST, Eric Blake wrote:
>> +    offset=$(($offset + 8))
>> +    bitmap=`peek_file_be "$TEST_IMG" $offset 8`
>> +
>> +    expected_bitmap=0
>> +    for bit in $expected_alloc; do
>> +        expected_bitmap=$(($expected_bitmap | (1 << $bit)))
>> +    done
>> +    for bit in $expected_zero; do
>> +        expected_bitmap=$(($expected_bitmap | (1 << (32 + $bit))))
>> +    done
>> +    expected_bitmap=`printf "%llu" $expected_bitmap`
>
> Dead statement - expected_bitmap is already a 64-bit decimal number
> without reprinting it to itself.

Not quite... it seems that simply expanding the variable treats the
value as signed so echo $((1 << 63)) returns INT64_MIN. The printf call
makes it unsigned, but even though I tried that in a 32-bit system and
it works now I'm actually wondering about the portability of the whole
thing.

Looking at the source it seems that bash uses intmax_t:

   https://git.savannah.gnu.org/cgit/bash.git/tree/variables.h?h=bash-5.0#n68

But if this is a problem then peek_file_* would also be affected, it
also uses printf %llu and a few iotests are already reading 64bit values
(grep 'peek_file_.* 8').

Berto