Message ID | 1390227608-7225-4-git-send-email-cyliu@suse.com |
---|---|
State | New |
Headers | show |
Am 20.01.2014 um 15:19 hat Chunyan Liu geschrieben: > Improve opt_get and opt_set group of functions. For opt_get, check and handle > NUlL input; for opt_set, when set to an existing option, rewrite the option > with new value. > > Signed-off-by: Dong Xu Wang <wdongxu@linux.vnet.ibm.com> > Signed-off-by: Chunyan Liu <cyliu@suse.com> Why do we want to allow NULL opts? Silently ignoring NULL instead of crashing leads to more subtle failure. Is there a legitimate user passing NULL? > util/qemu-option.c | 80 ++++++++++++++++++++++++++++++++++++++++++--------- > 1 files changed, 66 insertions(+), 14 deletions(-) > > diff --git a/util/qemu-option.c b/util/qemu-option.c > index fd84f95..8944b62 100644 > --- a/util/qemu-option.c > +++ b/util/qemu-option.c > @@ -499,6 +499,9 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) > { > QemuOpt *opt; > > + if (!opts) > + return NULL; The qemu coding style requires braces here (and in the following instances). > + > QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) { > if (strcmp(opt->name, name) != 0) > continue; > @@ -509,9 +512,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) > > const char *qemu_opt_get(QemuOpts *opts, const char *name) > { > - QemuOpt *opt = qemu_opt_find(opts, name); > + QemuOpt *opt; > const QemuOptDesc *desc; > > + if (!opts) > + return NULL; > + > + opt = qemu_opt_find(opts, name); > if (!opt) { > desc = find_desc_by_name(opts->list->desc, name); > if (desc && desc->def_value_str) { > @@ -535,10 +542,15 @@ bool qemu_opt_has_help_opt(QemuOpts *opts) > > bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) > { > - QemuOpt *opt = qemu_opt_find(opts, name); > + QemuOpt *opt; > const QemuOptDesc *desc; > Error *local_err = NULL; > > + if (!opts) > + return defval; > + > + opt = qemu_opt_find(opts, name); > + > if (opt == NULL) { > desc = find_desc_by_name(opts->list->desc, name); > if (desc && desc->def_value_str) { > @@ -553,10 +565,15 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) > > uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) > { > - QemuOpt *opt = qemu_opt_find(opts, name); > + QemuOpt *opt; > const QemuOptDesc *desc; > Error *local_err = NULL; > > + if (!opts) > + return defval; > + > + opt = qemu_opt_find(opts, name); > + > if (opt == NULL) { > desc = find_desc_by_name(opts->list->desc, name); > if (desc && desc->def_value_str) { > @@ -571,10 +588,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) > > uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) > { > - QemuOpt *opt = qemu_opt_find(opts, name); > + QemuOpt *opt; > const QemuOptDesc *desc; > Error *local_err = NULL; > > + if (!opts) > + return defval; > + > + opt = qemu_opt_find(opts, name); > if (opt == NULL) { > desc = find_desc_by_name(opts->list->desc, name); > if (desc && desc->def_value_str) { > @@ -612,6 +633,10 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp) > > static void qemu_opt_del(QemuOpt *opt) > { > + if (!opt) { > + return; > + } > + > QTAILQ_REMOVE(&opt->opts->head, opt, next); > g_free((/* !const */ char*)opt->name); > g_free((/* !const */ char*)opt->str); > @@ -664,6 +689,13 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, > return; > } > > + opt = qemu_opt_find(opts, name); > + if (opt) { > + g_free((char*)opt->str); > + opt->str = g_strdup(value); Why is qemu_opt_parse() not needed here? > + return; > + } > + > opt = g_malloc0(sizeof(*opt)); > opt->name = g_strdup(name); > opt->opts = opts; > @@ -704,16 +736,24 @@ 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; > + const QemuOptDesc *desc; > > - opt = g_malloc0(sizeof(*opt)); > - opt->desc = find_desc_by_name(desc, name); > - if (!opt->desc && !opts_accepts_any(opts)) { > + desc = find_desc_by_name(opts->list->desc, name); > + if (!desc && !opts_accepts_any(opts)) { > qerror_report(QERR_INVALID_PARAMETER, name); > - g_free(opt); > return -1; > } > > + opt = qemu_opt_find(opts, name); > + if (opt) { > + g_free((char*)opt->str); > + opt->value.boolean =val; Missing space after = > + opt->str = g_strdup(val ? "on" : "off"); > + return 0; > + } > + > + opt = g_malloc0(sizeof(*opt)); > + opt->desc = desc; > opt->name = g_strdup(name); > opt->opts = opts; > opt->value.boolean = !!val; Kevin
diff --git a/util/qemu-option.c b/util/qemu-option.c index fd84f95..8944b62 100644 --- a/util/qemu-option.c +++ b/util/qemu-option.c @@ -499,6 +499,9 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) { QemuOpt *opt; + if (!opts) + return NULL; + QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) { if (strcmp(opt->name, name) != 0) continue; @@ -509,9 +512,13 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char *name) const char *qemu_opt_get(QemuOpts *opts, const char *name) { - QemuOpt *opt = qemu_opt_find(opts, name); + QemuOpt *opt; const QemuOptDesc *desc; + if (!opts) + return NULL; + + opt = qemu_opt_find(opts, name); if (!opt) { desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { @@ -535,10 +542,15 @@ bool qemu_opt_has_help_opt(QemuOpts *opts) bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) { - QemuOpt *opt = qemu_opt_find(opts, name); + QemuOpt *opt; const QemuOptDesc *desc; Error *local_err = NULL; + if (!opts) + return defval; + + opt = qemu_opt_find(opts, name); + if (opt == NULL) { desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { @@ -553,10 +565,15 @@ bool qemu_opt_get_bool(QemuOpts *opts, const char *name, bool defval) uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) { - QemuOpt *opt = qemu_opt_find(opts, name); + QemuOpt *opt; const QemuOptDesc *desc; Error *local_err = NULL; + if (!opts) + return defval; + + opt = qemu_opt_find(opts, name); + if (opt == NULL) { desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { @@ -571,10 +588,14 @@ uint64_t qemu_opt_get_number(QemuOpts *opts, const char *name, uint64_t defval) uint64_t qemu_opt_get_size(QemuOpts *opts, const char *name, uint64_t defval) { - QemuOpt *opt = qemu_opt_find(opts, name); + QemuOpt *opt; const QemuOptDesc *desc; Error *local_err = NULL; + if (!opts) + return defval; + + opt = qemu_opt_find(opts, name); if (opt == NULL) { desc = find_desc_by_name(opts->list->desc, name); if (desc && desc->def_value_str) { @@ -612,6 +633,10 @@ static void qemu_opt_parse(QemuOpt *opt, Error **errp) static void qemu_opt_del(QemuOpt *opt) { + if (!opt) { + return; + } + QTAILQ_REMOVE(&opt->opts->head, opt, next); g_free((/* !const */ char*)opt->name); g_free((/* !const */ char*)opt->str); @@ -664,6 +689,13 @@ static void opt_set(QemuOpts *opts, const char *name, const char *value, return; } + opt = qemu_opt_find(opts, name); + if (opt) { + g_free((char*)opt->str); + opt->str = g_strdup(value); + return; + } + opt = g_malloc0(sizeof(*opt)); opt->name = g_strdup(name); opt->opts = opts; @@ -704,16 +736,24 @@ 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; + const QemuOptDesc *desc; - opt = g_malloc0(sizeof(*opt)); - opt->desc = find_desc_by_name(desc, name); - if (!opt->desc && !opts_accepts_any(opts)) { + desc = find_desc_by_name(opts->list->desc, name); + if (!desc && !opts_accepts_any(opts)) { qerror_report(QERR_INVALID_PARAMETER, name); - g_free(opt); return -1; } + opt = qemu_opt_find(opts, name); + if (opt) { + g_free((char*)opt->str); + opt->value.boolean =val; + opt->str = g_strdup(val ? "on" : "off"); + return 0; + } + + opt = g_malloc0(sizeof(*opt)); + opt->desc = desc; opt->name = g_strdup(name); opt->opts = opts; opt->value.boolean = !!val; @@ -726,16 +766,24 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, bool val) int qemu_opt_set_number(QemuOpts *opts, const char *name, int64_t val) { QemuOpt *opt; - const QemuOptDesc *desc = opts->list->desc; + const QemuOptDesc *desc; - opt = g_malloc0(sizeof(*opt)); - opt->desc = find_desc_by_name(desc, name); - if (!opt->desc && !opts_accepts_any(opts)) { + desc = find_desc_by_name(opts->list->desc, name); + if (!desc && !opts_accepts_any(opts)) { qerror_report(QERR_INVALID_PARAMETER, name); - g_free(opt); return -1; } + opt = qemu_opt_find(opts, name); + if (opt) { + g_free((char*)opt->str); + opt->value.uint = val; + opt->str = g_strdup_printf("%" PRId64, val); + return 0; + } + + opt = g_malloc0(sizeof(*opt)); + opt->desc = desc; opt->name = g_strdup(name); opt->opts = opts; opt->value.uint = val; @@ -870,6 +918,10 @@ void qemu_opts_del(QemuOpts *opts) { QemuOpt *opt; + if (!opts) { + return; + } + for (;;) { opt = QTAILQ_FIRST(&opts->head); if (opt == NULL)