[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-block] [PATCH v2 1/1] include: Add a comment to explain the or
From: |
Leonid Bloch |
Subject: |
Re: [Qemu-block] [PATCH v2 1/1] include: Add a comment to explain the origin of sizes' lookup table |
Date: |
Tue, 6 Nov 2018 08:56:08 +0000 |
Hi Phil, Hi Eric,
(Eric, for some reason you weren't CC'd to this thread - sorry.)
On 11/5/18 5:58 PM, Philippe Mathieu-Daudé wrote:
> Hi Leonid,
>
> On 4/11/18 19:07, Leonid Bloch wrote:
>> The lookup table for power-of-two sizes was added in commit 540b8492618eb
>> for the purpose of having convenient shortcuts for these sizes in cases
>> when the literal number has to be present at compile time, and
>> expressions as '(1 * KiB)' can not be used. One such case is the
>> stringification of sizes. Beyond that, it is convenient to use these
>> shortcuts for all power-of-two sizes, even if they don't have to be
>> literal numbers.
>>
>> Despite its convenience, this table introduced 55 lines of "dumb" code,
>> the purpose and origin of which are obscure without reading the message
>> of the commit which introduced it. This patch fixes that by adding a
>> comment to the code itself with a brief explanation for the reasoning
>> behind this table. This comment includes the short AWK script that
>> generated the table, so that anyone who's interested could make sure
>> that the values in it are correct (otherwise these values look as if
>> they were typed manually).
>>
>> Signed-off-by: Leonid Bloch <address@hidden>
>> ---
>> include/qemu/units.h | 18 ++++++++++++++++++
>> 1 file changed, 18 insertions(+)
>>
>> diff --git a/include/qemu/units.h b/include/qemu/units.h
>> index 68a7758650..1c959d182e 100644
>> --- a/include/qemu/units.h
>> +++ b/include/qemu/units.h
>> @@ -17,6 +17,24 @@
>> #define PiB (INT64_C(1) << 50)
>> #define EiB (INT64_C(1) << 60)
>> +/*
>> + * The following lookup table is intended to be used when a literal
>> string of
>> + * the number of bytes is required (for example if it needs to be
>> stringified).
>> + * It can also be used for generic shortcuts of power-of-two sizes.
>
> ^ ok
>
> v this part is not useful in the tree IMHO.
>
>> + * This table is generated using the AWK script below:
>> + *
>> + * BEGIN {
>> + * suffix="KMGTPE";
>> + * for(i=10; i<64; i++) {
>> + * val=2**i;
>> + * s=substr(suffix, int(i/10), 1);
>> + * n=2**(i%10);
>> + * pad=21-int(log(n)/log(10));
>> + * printf("#define S_%d%siB %*d\n", n, s, pad, val);
>> + * }
>> + * }
>> + */
>
> If it is generated and the stringified are also generated, why not
> generate this once via the ./configure, since it is used by machines and
> not by humans?
You beat me to it! I was just thinking about that last evening. Indeed
it's not so elegant to put generated code in a file that is intended for
human handling. Generating by ./configure would be prettier.
A runtime solution that interprets hard-coded expression strings, like
Eric suggested, can also be a possibility, but it seems more complicated
than just generating these at the configuration stage. Plus one gets the
S_* constants that can be used in many places, not only where an
explicit number is needed. What do you think?
A question though: if it to be generated by ./configure, where do you
suggest to put the generated values? And also: is it OK to assume
there's AWK (or another scripting language) on the system for generating
these?
Thanks,
Leonid.