Anthony Liguori <aligu...@us.ibm.com> writes: > We don't use the standard C functions for conversion because we don't want to > depend on the user's locale. All option names in QEMU are en_US in plain > ASCII.
Fails to explain the important part, namely the actual change to option comparison! > Signed-off-by: Anthony Liguori <aligu...@us.ibm.com> > --- > qemu-option.c | 53 +++++++++++++++++++++++++++++++++++++++++++++-------- > qemu-option.h | 2 ++ > 2 files changed, 47 insertions(+), 8 deletions(-) > > diff --git a/qemu-option.c b/qemu-option.c > index 8334190..6494c99 100644 > --- a/qemu-option.c > +++ b/qemu-option.c > @@ -89,6 +89,43 @@ const char *get_opt_value(char *buf, int buf_size, const > char *p) > return p; > } > > +static int opt_tolower(int ch) > +{ > + if (ch >= 'A' && ch <= 'Z') { > + return 'a' + (ch - 'A'); > + } else if (ch == '_') { > + return '-'; > + } > + > + return ch; > +} > + > +int qemu_opt_name_cmp(const char *lhs, const char *rhs) > +{ > + int i; > + > + g_assert(lhs && rhs); Why bother? > + > + for (i = 0; lhs[i] && rhs[i]; i++) { > + int ret; > + > + ret = opt_tolower(lhs[i]) - opt_tolower(rhs[i]); > + if (ret < 0) { > + return -1; > + } else if (ret > 0) { > + return 1; > + } > + } > + > + if (!lhs[i] && rhs[i]) { > + return -1; > + } else if (lhs[i] && !rhs[i]) { > + return 1; > + } > + > + return 0; > +} > + Are you sure you want to make options case-insensitive? If yes, please mention it in the commit message. The implementation is too longwinded for my taste :) static unsigned char opt_canon_ch(char ch) { if (ch >= 'A' && ch <= 'Z') { return 'a' + (ch - 'A'); } else if (ch == '_') { return '-'; } return ch; } int qemu_opt_name_cmp(const char *lhs, const char *rhs) { unsigned char l, r; do { l = opt_canon_ch(*lhs++); r = opt_canon_ch(*rhs++); } while (l && l == r); return l - r; } Or maybe a bool qemu_opt_name_eq() instead. > int get_next_param_value(char *buf, int buf_size, > const char *tag, const char **pstr) > { > @@ -101,7 +138,7 @@ int get_next_param_value(char *buf, int buf_size, > if (*p != '=') > break; > p++; > - if (!strcmp(tag, option)) { > + if (!qemu_opt_name_cmp(tag, option)) { > *pstr = get_opt_value(buf, buf_size, p); > if (**pstr == ',') { > (*pstr)++; Aha, you cover the non-QemuOpts stuff, too. Fine with me. > @@ -137,7 +174,7 @@ int check_params(char *buf, int buf_size, > } > p++; > for (i = 0; params[i] != NULL; i++) { > - if (!strcmp(params[i], buf)) { > + if (!qemu_opt_name_cmp(params[i], buf)) { > break; > } > } > @@ -160,7 +197,7 @@ QEMUOptionParameter > *get_option_parameter(QEMUOptionParameter *list, > const char *name) > { > while (list && list->name) { > - if (!strcmp(list->name, name)) { > + if (!qemu_opt_name_cmp(list->name, name)) { > return list; > } > list++; > @@ -516,7 +553,7 @@ static QemuOpt *qemu_opt_find(QemuOpts *opts, const char > *name) > QemuOpt *opt; > > QTAILQ_FOREACH_REVERSE(opt, &opts->head, QemuOptHead, next) { > - if (strcmp(opt->name, name) != 0) > + if (qemu_opt_name_cmp(opt->name, name) != 0) > continue; > return opt; > } > @@ -599,7 +636,7 @@ static void opt_set(QemuOpts *opts, const char *name, > const char *value, > int i; > > for (i = 0; desc[i].name != NULL; i++) { > - if (strcmp(desc[i].name, name) == 0) { > + if (qemu_opt_name_cmp(desc[i].name, name) == 0) { > break; > } > } > @@ -660,7 +697,7 @@ int qemu_opt_set_bool(QemuOpts *opts, const char *name, > bool val) > int i; > > for (i = 0; desc[i].name != NULL; i++) { > - if (strcmp(desc[i].name, name) == 0) { > + if (qemu_opt_name_cmp(desc[i].name, name) == 0) { > break; > } > } > @@ -709,7 +746,7 @@ QemuOpts *qemu_opts_find(QemuOptsList *list, const char > *id) > } > continue; > } > - if (strcmp(opts->id, id) != 0) { > + if (qemu_opt_name_cmp(opts->id, id) != 0) { > continue; > } > return opts; There's a strcmp(option, "id") in opts_do_parse(), and a similar one in qemu_opts_from_qdict_1(). Perhaps they should be compared with qemu_opt_name_cmp(), too, just for consistency. > @@ -1062,7 +1099,7 @@ void qemu_opts_validate(QemuOpts *opts, const > QemuOptDesc *desc, Error **errp) > int i; > > for (i = 0; desc[i].name != NULL; i++) { > - if (strcmp(desc[i].name, opt->name) == 0) { > + if (qemu_opt_name_cmp(desc[i].name, opt->name) == 0) { > break; > } > } > diff --git a/qemu-option.h b/qemu-option.h > index 951dec3..ee27a13 100644 > --- a/qemu-option.h > +++ b/qemu-option.h > @@ -141,4 +141,6 @@ int qemu_opts_print(QemuOpts *opts, void *dummy); > int qemu_opts_foreach(QemuOptsList *list, qemu_opts_loopfunc func, void > *opaque, > int abort_on_failure); > > +int qemu_opt_name_cmp(const char *lhs, const char *rhs); > + > #endif Why external?