diff options
author | Dong Xu Wang <wdongxu@linux.vnet.ibm.com> | 2012-12-06 14:47:20 +0800 |
---|---|---|
committer | Kevin Wolf <kwolf@redhat.com> | 2012-12-11 16:33:27 +0100 |
commit | ad718d01ba0af531d10b0a8685cf5047edfd1891 (patch) | |
tree | 7548067ce1468e57b6e98232ab9b0d0ee2e90586 /qemu-option.c | |
parent | db97ceba1e17db59188e91b66e61bf84a6a71081 (diff) | |
download | qemu-ad718d01ba0af531d10b0a8685cf5047edfd1891.zip qemu-ad718d01ba0af531d10b0a8685cf5047edfd1891.tar.gz qemu-ad718d01ba0af531d10b0a8685cf5047edfd1891.tar.bz2 |
qemu-option: qemu_opt_set_bool(): fix code duplication
It will set opt->str in qemu_opt_set_bool, without opt->str, there
will be some potential bugs.
These are uses of opt->str, and what happens when it isn't set:
* qemu_opt_get(): returns NULL, which means "not set". Bug can bite
when value isn't the default value.
* qemu_opt_parse(): passes NULL to parse_option_bool(), which treats it
like "on". Wrong if the value is actually false. Bug can bite when
qemu_opts_validate() runs after qemu_opt_set_bool().
* qemu_opt_del(): passes NULL to g_free(), which is just fine.
* qemu_opt_foreach(): passes NULL to the callback, which is unlikely to
be prepared for it.
* qemu_opts_print(): prints NULL, which crashes on some systems.
* qemu_opts_to_qdict(): passes NULL to qstring_from_str(), which
crashes.
It also makes qemu_opt_set_bool more readable by using find_desc_by_name
and opts_accepts_any.
It is based on Luiz's patch and uses Markus's comments. Discussions can
be found at:
http://lists.nongnu.org/archive/html/qemu-devel/2012-07/msg02716.html
Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com>
Signed-off-by: Kevin Wolf <kwolf@redhat.com>
Diffstat (limited to 'qemu-option.c')
-rw-r--r-- | qemu-option.c | 27 |
1 files changed, 9 insertions, 18 deletions
diff --git a/qemu-option.c b/qemu-option.c index 74321bb..e0131ce 100644 --- a/qemu-option.c +++ b/qemu-option.c @@ -677,30 +677,21 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) { QemuOpt *opt; const QemuOptDesc *desc = opts->list->desc; - int i; - for (i = 0; desc[i].name != NULL; i++) { - if (strcmp(desc[i].name, name) == 0) { - break; - } - } - if (desc[i].name == NULL) { - if (i == 0) { - /* empty list -> allow any */; - } else { - qerror_report(QERR_INVALID_PARAMETER, name); - return -1; - } + opt = g_malloc0(sizeof(*opt)); + opt->desc = find_desc_by_name(desc, name); + if (!opt->desc && !opts_accepts_any(opts)) { + qerror_report(QERR_INVALID_PARAMETER, name); + g_free(opt); + return -1; } - opt = g_malloc0(sizeof(*opt)); opt->name = g_strdup(name); opt->opts = opts; - QTAILQ_INSERT_TAIL(&opts->head, opt, next); - if (desc[i].name != NULL) { - opt->desc = desc+i; - } opt->value.boolean = !!val; + opt->str = g_strdup(val ? "on" : "off"); + QTAILQ_INSERT_TAIL(&opts->head, opt, next); + return 0; } |