[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code
From: |
Paolo Bonzini |
Subject: |
Re: [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication |
Date: |
Thu, 27 Sep 2012 13:55:18 +0200 |
User-agent: |
Mozilla/5.0 (X11; Linux x86_64; rv:15.0) Gecko/20120911 Thunderbird/15.0.1 |
Il 27/09/2012 07:14, Dong Xu Wang ha scritto:
> Call qemu_opt_set() instead of duplicating opt_set().
>
> 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.
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> Signed-off-by: Dong Xu Wang <address@hidden>
> ---
> qemu-option.c | 28 +---------------------------
> 1 files changed, 1 insertions(+), 27 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index 27891e7..0b81e77 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -667,33 +667,7 @@ void qemu_opt_set_err(QemuOpts *opts, const char *name,
> const char *value,
>
> 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->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;
> - return 0;
> + return qemu_opt_set(opts, name, val ? "on" : "off");
This now calls qemu_opt_parse, which won't work if you have opt->desc =
NULL.
Instead of this patch, using the new functions you create in patch 2
should be enough to limit the code duplication in qemu_opt_set_bool.
Paolo
> }
>
> int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
>
- [Qemu-devel] [RFC v2 0/7] replace QEMUOptionParameter with QemuOpts parser, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication, Dong Xu Wang, 2012/09/27
- Re: [Qemu-devel] [RFC v2 1/7] qemu-option: qemu_opt_set_bool(): fix code duplication,
Paolo Bonzini <=
- [Qemu-devel] [RFC v2 2/7] qemu-option: opt_set(): split it up into more functions, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 3/7] qemu-option: qemu_opts_validate(): fix duplicated code, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 4/7] introduce qemu_opts_create_nofail function, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 5/7] use qemu_opts_create_nofail, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 6/7] create new function: qemu_opt_set_number, Dong Xu Wang, 2012/09/27
- [Qemu-devel] [RFC v2 7/7] remove QEMUOptionParameter, Dong Xu Wang, 2012/09/27