On 5/29/20 10:07 AM, Alberto Garcia wrote:
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,

Ah, yes, then that makes sense. Still, you could shave a fork or comment the action by doing:

printf -v expected_bitmap %llu $expected_bitmap # convert to 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.

Bash supports 64-bit numbers even on 32-bit platforms, and has done for years. Since we are running the test only under bash, that's all the more we have to worry about.


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').

In cases where a negative number is read but the 64-bit pattern is the same, it doesn't matter; but here, you are using it for output, and so you do want the unsigned representation instead of bash's default signed representation.

--
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


Reply via email to