Eric Blake <ebl...@redhat.com> writes:
> On 02/21/2017 03:01 PM, Markus Armbruster wrote:
>> From: "Daniel P. Berrange" <berra...@redhat.com>
>>
>> Currently the QObjectInputVisitor assumes that all scalar values are
>> directly represented as the final types declared by the thing being
>> visited. i.e. it assumes an 'int' is using QInt, and a 'bool' is using
>> QBool, etc. This is good when QObjectInputVisitor is fed a QObject
>> that came from a JSON document on the QMP monitor, as it will strictly
>> validate correctness.
>>
>> To allow QObjectInputVisitor to be reused for visiting a QObject
>> originating from keyval_parse(), an alternative mode is needed where
>> all the scalars types are represented as QString and converted on the
>> fly to the final desired type.
>>
>> Signed-off-by: Daniel P. Berrange <berra...@redhat.com>
>> Message-Id: <1475246744-29302-8-git-send-email-berra...@redhat.com>
>>
>> Rebased, conflicts resolved, commit message updated to refer to
>> keyval_parse().
>>
>> Control flow in qobject_input_type_number_autocast() simplified.
>>
>> Additional tests in test-qemu-opts.c to verify QemuOpts compatibility.
>> To make the tests pass, use qemu_strtou64() instead of
>> parse_uint_full().
>>
>> Use qemu_strtou64() and qemu_strtosz() instead of
>> parse_option_number() and parse_option_size() so we have to call
>> qobject_input_get_name() only when actually needed.
>>
>> Signed-off-by: Markus Armbruster <arm...@redhat.com>
>> ---
>> include/qapi/qobject-input-visitor.h | 32 ++++-
>> qapi/qobject-input-visitor.c | 165 ++++++++++++++++++++++++
>> tests/test-keyval.c | 241
>> +++++++++++++++++++++++++++++++++++
>> tests/test-qobject-input-visitor.c | 194 +++++++++++++++++++++++++++-
>> 4 files changed, 624 insertions(+), 8 deletions(-)
>>
>
>> @@ -71,6 +72,7 @@ static const char
>> *qobject_input_get_name(QObjectInputVisitor *qiv,
>> g_string_prepend(qiv->errname, name);
>> g_string_prepend_c(qiv->errname, '.');
>> } else {
>> + /* TODO needs to be .%zu for autocast */
>> snprintf(buf, sizeof(buf), "[%zu]", so->index);
>
> Ah, the TODO means you started thinking about list integration, and
> that's part of why this is still RFC.
>
>
>> +static void qobject_input_type_bool_autocast(Visitor *v, const char *name,
>> + bool *obj, Error **errp)
>> +{
>> + QObjectInputVisitor *qiv = to_qiv(v);
>> + QObject *qobj = qobject_input_get_object(qiv, name, true, errp);
>> + QString *qstr;
>> + const char *str;
>> +
>> + if (!qobj) {
>> + return;
>> + }
>> + qstr = qobject_to_qstring(qobj);
>> + if (!qstr) {
>> + error_setg(errp, QERR_INVALID_PARAMETER_TYPE,
>> + qobject_input_get_name(qiv, name), "string");
>> + return;
>> + }
>> +
>> + str = qstring_get_str(qstr);
>> + if (!strcmp(str, "on")) {
>> + *obj = true;
>> + } else if (!strcmp(str, "off")) {
>> + *obj = false;
>
> Why are we reimplementing only a subset of parse_option_bool() here?
We're reimplementing parse_option_bool() exactly:
static void parse_option_bool(const char *name, const char *value, bool
*ret,
Error **errp)
{
if (!strcmp(value, "on")) {
*ret = 1;
} else if (!strcmp(value, "off")) {
*ret = 0;
} else {
error_setg(errp, QERR_INVALID_PARAMETER_VALUE,
name, "'on' or 'off'");
}
}
You're probably thinking of opts_type_bool(), which recognizes
additional spellings. But options visitor compatibility is a headache
left for later.
> Overall, most of the changes from Dan's original look sane.
Thanks!
