Re: [Qemu-trivial] [Qemu-devel] [PATCH v5 01/46] include: Add IEC binary prefixes in "qemu/units.h"

[Philippe Mathieu-Daudé]

On 06/27/2018 09:26 AM, Eric Blake wrote:
> On 06/27/2018 06:27 AM, Igor Mammedov wrote:
>> On Mon, 25 Jun 2018 09:41:53 -0300
>> Philippe Mathieu-Daudé <address@hidden> wrote:
>>
>>> Loosely based on 076b35b5a56.
>>>
>>> Suggested-by: Stefan Weil <address@hidden>
>>> Signed-off-by: Philippe Mathieu-Daudé <address@hidden>
>>> ---
> 
>>> +#ifndef QEMU_UNITS_H
>>> +#define QEMU_UNITS_H
>>> +
>>> +#define KiB     (INT64_C(1) << 10)
>>> +#define MiB     (INT64_C(1) << 20)
>>> +#define GiB     (INT64_C(1) << 30)
>>> +#define TiB     (INT64_C(1) << 40)
>>> +#define PiB     (INT64_C(1) << 50)
>>> +#define EiB     (INT64_C(1) << 60)
>> Shouldn't above use UINT64_C()
> 
> Since the decision of signed vs. unsigned was intentional based on
> review on earlier versions, it may be worth a comment in this file that
> these constants are intentionally signed (in usage patterns, these tend
> to be multiplied by another value; and while it is easy to go to
> unsigned by doing '1U * KiB', you can't go in the opposite direction if
> you want a signed number for '1 * KiB' unless KiB is signed).

OK.

I'll also change this tests using your suggestion:

diff --git a/tests/test-qemu-opts.c b/tests/test-qemu-opts.c
@@ -709,8 +709,7 @@ static void test_opts_parse_size(void)
                            false, &error_abort);
-    g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0),
-                     ==, 16777215 * T_BYTE);
+    g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215U
* TiB);

to avoid this error on 32-bit archs:

source/qemu/tests/test-qemu-opts.c: In function 'test_opts_parse_size':
source/qemu/tests/test-qemu-opts.c:713:71: error: integer overflow in
expression [-Werror=overflow]
  g_assert_cmphex(qemu_opt_get_size(opts, "size2", 0), ==, 16777215 * TiB);
                                                                       ^