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 <ebl...@redhat.com> --- 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; diff --git a/util/qemu-option.c b/util/qemu-option.c index de42e2a406a..06c4e8102a8 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -321,7 +321,8 @@ const char *qemu_opt_get(QemuOpts *opts, const char *name) opt = qemu_opt_find(opts, name); if (!opt) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); - if (desc && desc->def_value_str) { + if (desc) { + assert(!desc->def_value_int); return desc->def_value_str; } } @@ -364,8 +365,22 @@ char *qemu_opt_get_del(QemuOpts *opts, const char *name) opt = qemu_opt_find(opts, name); if (!opt) { desc = find_desc_by_name(opts->list->desc, name); - if (desc && desc->def_value_str) { - str = g_strdup(desc->def_value_str); + if (desc) { + if (desc->def_value_str) { + str = g_strdup(desc->def_value_str); + } else if (desc->def_value_int) { + switch (desc->type) { + case QEMU_OPT_BOOL: + str = g_strdup("on"); + break; + case QEMU_OPT_NUMBER: + case QEMU_OPT_SIZE: + str = g_strdup_printf("%" PRId64, desc->def_value_int); + break; + default: + abort(); + } + } } return str; } @@ -400,8 +415,15 @@ static bool qemu_opt_get_bool_helper(QemuOpts *opts, const char *name, opt = qemu_opt_find(opts, name); if (opt == NULL) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); - if (desc && desc->def_value_str) { - parse_option_bool(name, desc->def_value_str, &ret, &error_abort); + if (desc) { + if (desc->def_value_int) { + assert(desc->type != QEMU_OPT_STRING); + return true; + } + if (desc->def_value_str) { + parse_option_bool(name, desc->def_value_str, &ret, + &error_abort); + } } return ret; } @@ -436,8 +458,15 @@ static uint64_t qemu_opt_get_number_helper(QemuOpts *opts, const char *name, opt = qemu_opt_find(opts, name); if (opt == NULL) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); - if (desc && desc->def_value_str) { - parse_option_number(name, desc->def_value_str, &ret, &error_abort); + if (desc) { + if (desc->def_value_int) { + assert(desc->type != QEMU_OPT_STRING); + return desc->def_value_int; + } + if (desc->def_value_str) { + parse_option_number(name, desc->def_value_str, &ret, + &error_abort); + } } return ret; } @@ -473,8 +502,15 @@ static uint64_t qemu_opt_get_size_helper(QemuOpts *opts, const char *name, opt = qemu_opt_find(opts, name); if (opt == NULL) { const QemuOptDesc *desc = find_desc_by_name(opts->list->desc, name); - if (desc && desc->def_value_str) { - parse_option_size(name, desc->def_value_str, &ret, &error_abort); + if (desc) { + if (desc->def_value_int) { + assert(desc->type != QEMU_OPT_STRING); + return desc->def_value_int; + } + if (desc->def_value_str) { + parse_option_size(name, desc->def_value_str, &ret, + &error_abort); + } } return ret; } @@ -787,9 +823,23 @@ void qemu_opts_print(QemuOpts *opts, const char *separator) } for (; desc && desc->name; desc++) { const char *value; + char *tmp = NULL; opt = qemu_opt_find(opts, desc->name); value = opt ? opt->str : desc->def_value_str; + if (!value && desc->def_value_int) { + switch (desc->type) { + case QEMU_OPT_BOOL: + value = tmp = g_strdup("on"); + break; + case QEMU_OPT_NUMBER: + case QEMU_OPT_SIZE: + value = tmp = g_strdup_printf("%" PRId64, desc->def_value_int); + break; + default: + abort(); + } + } if (!value) { continue; } @@ -803,6 +853,7 @@ void qemu_opts_print(QemuOpts *opts, const char *separator) printf("%s%s=%s", sep, desc->name, value); } sep = separator; + g_free(tmp); } } -- 2.20.1