Kevin Wolf <[email protected]> writes: > Am 14.04.2020 um 11:42 hat Markus Armbruster geschrieben: >> Eric Blake <[email protected]> writes: >> >> > On 4/9/20 10:30 AM, Markus Armbruster wrote: >> >> The next commits will put it to use. >> >> >> >> Signed-off-by: Markus Armbruster <[email protected]> >> >> --- >> >> util/qemu-option.c | 102 +++++++++++++++++++++++++-------------------- >> >> 1 file changed, 56 insertions(+), 46 deletions(-) >> >> >> > >> >> +static const char *get_opt_name_value(const char *params, >> >> + const char *firstname, >> >> + char **name, char **value) >> >> +{ >> >> + const char *p, *pe, *pc; >> >> + >> >> + pe = strchr(params, '='); >> >> + pc = strchr(params, ','); >> >> + >> >> + if (!pe || (pc && pc < pe)) { >> >> + /* found "foo,more" */ >> >> + if (firstname) { >> >> + /* implicitly named first option */ >> >> + *name = g_strdup(firstname); >> >> + p = get_opt_value(params, value); >> > >> > Is this correct even when params is "foo,,more"? But... >> > >> >> static void opts_do_parse(QemuOpts *opts, const char *params, >> >> const char *firstname, bool prepend, >> >> bool *invalidp, Error **errp) >> >> { >> >> - char *option = NULL; >> >> - char *value = NULL; >> >> - const char *p,*pe,*pc; >> >> Error *local_err = NULL; >> >> + char *option, *value; >> >> + const char *p; >> >> - for (p = params; *p != '\0'; p++) { >> >> - pe = strchr(p, '='); >> >> - pc = strchr(p, ','); >> >> - if (!pe || (pc && pc < pe)) { >> >> - /* found "foo,more" */ >> >> - if (p == params && firstname) { >> >> - /* implicitly named first option */ >> >> - option = g_strdup(firstname); >> >> - p = get_opt_value(p, &value); >> > >> > ...in this patch, it is just code motion, so if it is a bug, it's >> > pre-existing and worth a separate fix. >> >> If @p points to "foo,,more", possibly followed by additional characters, >> then we have pc && pc < pe, and enter this conditional. This means that >> >> foo,,more=42 >> >> gets parsed as two name=value, either as >> >> name value >> ----------------------- >> @firstname "foo,more" >> "" "42" >> >> if @firstname is non-null > > Hm, how that? get_opt_value() doesn't stop at '=', so shouldn't you get > a single option @firstname with a value of "foo,more=42"? > > name value > ----------------------- > @firstname "foo,more=42" > >> or else as >> >> name value >> ----------------- >> "foo,,more" "on" >> "" "42" > > Here get_opt_name() stops at the first comma. You get a positive flag > "foo" and the remaing option string starts with a comma, so the > condition will still be true for the next loop iteration. Now you get a > positive flag with an empty name "". Finally, the rest is parsed as an > option, so we get: > > name value > ----------------------- > "foo" "on" > "" "on" > "more" "42" > > Actually, at this point I thought I could just check, and gdb confirms > my reading for both cases.
You're right. I should know better than trying to predict what the QemuOpts parser does. > This is still crazy, but perhaps less so than the interpretations you > suggested. Permitting anti-social characters in the NAME part of NAME=VALUE is crazy. To findout which kind of crazy exactly, run the code and observe. Do not blindly trust the maintainer's explanations, because he's quite possibly just as confused as everybody else.
