Am 14.04.2020 um 11:42 hat Markus Armbruster geschrieben:
> Eric Blake <ebl...@redhat.com> writes:
>
> > On 4/9/20 10:30 AM, Markus Armbruster wrote:
> >> The next commits will put it to use.
> >>
> >> Signed-off-by: Markus Armbruster <arm...@redhat.com>
> >> ---
> >> 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.
This is still crazy, but perhaps less so than the interpretations you
suggested.
Kevin
