[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Qemu-devel] [PATCH 1/8] qemu-option: qemu_opt_set_bool(): fix code
From: |
Markus Armbruster |
Subject: |
Re: [Qemu-devel] [PATCH 1/8] qemu-option: qemu_opt_set_bool(): fix code duplication |
Date: |
Sat, 21 Jul 2012 10:09:09 +0200 |
User-agent: |
Gnus/5.13 (Gnus v5.13) Emacs/24.0.97 (gnu/linux) |
Luiz Capitulino <address@hidden> writes:
> Call qemu_opt_set() instead of duplicating opt_set().
>
> Signed-off-by: Luiz Capitulino <address@hidden>
> ---
> qemu-option.c | 28 +---------------------------
> 1 file changed, 1 insertion(+), 27 deletions(-)
>
> diff --git a/qemu-option.c b/qemu-option.c
> index bb3886c..2cb2835 100644
> --- a/qemu-option.c
> +++ b/qemu-option.c
> @@ -677,33 +677,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");
> }
>
> int qemu_opt_foreach(QemuOpts *opts, qemu_opt_loopfunc func, void *opaque,
Does a bit more than just obvious code de-duplication. Two things in
particular:
* Error reporting
Old: qerror_report(); return -1
New: opt_set() and qemu_opt_set() cooperate, like this:
opt_set(): error_set(); return;
qemu_opt_set():
if (error_is_set(&local_err)) {
qerror_report_err(local_err);
error_free(local_err);
return -1;
}
I guess the net effect is the same. Not sure it's worth mentioning in
the commit message.
* New sets opt->str to either "on" or "off" depending on val, then lets
reconstructs the value with qemu_opt_parse(). Which can't fail then.
I figure the latter part has no further impact. But what about
setting opt->str? Is this a bug fix?
[Qemu-devel] [PATCH 2/8] qemu-option: opt_set(): split it up into more functions, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 3/8] qemu-option: qemu_opts_validate(): fix duplicated code, Luiz Capitulino, 2012/07/13
[Qemu-devel] [PATCH 4/8] qemu-option: add alias support, Luiz Capitulino, 2012/07/13