Re: [Qemu-devel] [PATCH v3 1/6] qemu-option: Allow integer defaults

[Leonid Bloch]

On 1/11/19 6:23 PM, Eric Blake wrote:
> On 1/11/19 8:14 AM, Leonid Bloch wrote:
>> On 1/10/19 9:18 PM, Eric Blake wrote:
>>> Set the framework up for declaring integer options with an integer
>>> default, instead of our current insane approach of requiring the
>>> default value to be given as a string (which then has to be reparsed
>>> at every use that wants a number).  git grep '[^.]def_value_str' says
>>> that we have done a good job of NOT abusing the internal field
>>> outside of the implementation in qemu-option.c; therefore, it is not
>>> too hard to audit that all code can either handle the new integer
>>> defaults correctly or abort because a caller violated constraints.
>>> Sadly, we DO have a constraint that qemu_opt_get() should not be
>>> called on an option that has an integer type, because we have no
>>> where to stash a cached const char * result; but callers that want
>>> an integer should be using qemu_opt_get_number() and friends
>>> anyways.
>>>
>>> Signed-off-by: Eric Blake <address@hidden>
>>> ---
>>>    include/qemu/option.h | 12 ++++++++
>>>    util/qemu-option.c    | 69 +++++++++++++++++++++++++++++++++++++------
>>>    2 files changed, 72 insertions(+), 9 deletions(-)
>>>
>>> diff --git a/include/qemu/option.h b/include/qemu/option.h
>>> index 844587cab39..46b80d5a6e1 100644
>>> --- a/include/qemu/option.h
>>> +++ b/include/qemu/option.h
>>> @@ -46,6 +46,18 @@ typedef struct QemuOptDesc {
>>>        const char *name;
>>>        enum QemuOptType type;
>>>        const char *help;
>>> +    /*
>>> +     * For QEMU_OPT_STRING: Leave def_value_int 0, and set def_value_str
>>> +     * to a default value or leave NULL for no default.
>>> +     *
>>> +     * For other types: Initialize at most non-zero def_value_int or a
>>> +     * parseable def_value_str for a default (must use a string for an
>>> +     * explicit default of 0, although an implicit default generally
>>> +     * works).  If setting def_value_int, calling qemu_opt_get() on
>>> +     * that option will abort(); instead, call qemu_opt_get_del() or a
>>> +     * typed getter.
>>> +     */
>>> +    uint64_t def_value_int;
>>>        const char *def_value_str;
>>>    } QemuOptDesc;
>>>
> 
>>
>> If I understand correctly, the number will still be compiled into the
>> binary as an expression string, but will be printed correctly during
>> runtime?
> 
> Anyone that uses .def_value_int will compile into the binary as an
> 8-byte integer (regardless of how that number was spelled in C, either
> as a single constant, or as a constant expression), and NOT as a decimal
> string.  String conversions for code that asks for a string will happen
> by a runtime printf() (ideally, such code is limited to the case of
> printing out information during help output).  Code that is smart enough
> to request a number now gets the default value directly, rather than the
> old way of having to strtoll() decode a decimal string back into a
> number.  No one should ever be using .def_value_str = stringify(macro),
> when they can instead just use .def_value_int = macro (which is what the
> next patch fixes).

Yes, you're right. Thanks for the explanation!