Re: [Qemu-block] [Qemu-devel] [PATCH v2 1/1] include: Auto-generate the sizes lookup table

[Markus Armbruster]

Leonid Bloch <address@hidden> writes:

> Hi,
>
> On 1/8/19 2:20 PM, Kevin Wolf wrote:
>> Am 03.01.2019 um 22:33 hat Leonid Bloch geschrieben:
>>> The lookup table for power-of-two sizes is now auto-generated during the
>>> build, and not hard-coded into the units.h file.
>>>
>>> This partially reverts commit 540b8492618eb.
>>>
>>> Signed-off-by: Leonid Bloch <address@hidden>
>> 
>> During a downstream review, Max found a problem with the table that we
>> could fix while we're touching it:
>> 
>>      Upstream: All >= S_2GiB are not valid ints.  (qemu assumes that
>>      sizeof(int) == 4, right?)  So S_2GiB should be 2147483648u and all
>>      above should be ...ull or better UINT64_C().

So their (integer) type can vary.  Whether that's a problem depends on
how the macros are used.

> But the initial reasoning for this table was to have a pure number 
> there. If there will be strings like "2147483648u/ull" or 
> "UINT64_C(...)" there, they will be stringified, literally, and will 
> appear as such inside the binary.

"Macro that expands to an integer constant that stringify() turns into a
decimal number" is a rather peculiar contract.

>                                   If specifying the unit64 type is 
> really needed, one can always use, e.g., 2 * GiB, from units.h.

Right now I suspect these S_ macros should generally be avoided.

We have 54 of them.  I count six uses:

    block/qcow2.h:#define QCOW_MAX_REFTABLE_SIZE S_8MiB
    block/qcow2.h:#define QCOW_MAX_L1_SIZE S_32MiB
    block/qcow2.h:#define DEFAULT_L2_CACHE_MAX_SIZE S_32MiB
    block/qcow2.h:#define DEFAULT_L2_CACHE_MAX_SIZE S_8MiB
    block/qcow2.h:#define DEFAULT_CLUSTER_SIZE S_64KiB
    block/vdi.c:#define DEFAULT_CLUSTER_SIZE S_1MiB

Which if them truly need stringify-able integers?