Eric Blake <ebl...@redhat.com> writes: > On 02/28/2017 09:48 AM, Kevin Wolf wrote: >> Am 27.02.2017 um 12:20 hat Markus Armbruster geschrieben: >>> keyval_parse() parses KEY=VALUE,... into a QDict. Works like >>> qemu_opts_parse(), except: >>> > >>> + >>> +/* >>> + * KEY=VALUE,... syntax: >>> + * >>> + * key-vals = [ key-val { ',' key-vals } ] > > Just refreshing my memory: in this grammar, [] means optional (0 or 1), > and {} means repeating (0 or more).
Yes. > That means an empty string satisfies key-vals (as in "-option ''"), > intentional? Yes. Creates an empty root object. If we don't want that, or don't want it now, I can make this case an error. > I don't see how this permits a trailing comma, but isn't that one of > your goals to allow "-option key=val," the same as "-option key=val"? Mistake. Possible fix: key-vals = [ key-val { ',' key-val } [ ',' ] >>> + * key-val = key '=' val >>> + * key = key-fragment { '.' key-fragment } > > Ambiguous. I'm dense. Can you give me an example string with two derivations? >>> + * key-fragment = / [^=,.]* / > > Do you want + instead of * in the regex, so as to require a non-empty > string for key-fragment? After all, you want to reject "-option a..b=val". I like to keep syntactic and semantic analysis conceptually separate. keyval_parse() looks for the next '.' to extract a key-fragment (syntactic analysis). It then rejects key-fragments it doesn't like (semantic analysis). Right now, it only dislikes lengths outside [1,127]. Later on, it'll additionally dislike key-fragments that are neither valid QAPI names nor digit strings. Perhaps my comment could explain this better. >>> + * val = { / [^,]* / | ',,' } > > Here, * makes sense, since an empty value is permitted in '-option key=". > >>> + * >>> + * Semantics defined by reduction to JSON: >>> + * >>> + * key-vals defines a tree of objects rooted at R >>> + * where for each key-val = key-fragment . ... = val in key-vals >>> + * R op key-fragment op ... = val' >>> + * where (left-associative) op is member reference L.key-fragment >> >> Maybe it's just me, but I can't say that I fully understand what these >> last two lines are supposed to tell me. > > I think it's trying to portray dictionary member lookup semantics (each > key-fragment represents another member lookup one dictionary deeper, > before reaching the final lookup to the scalar value) - but yeah, it was > a confusing read to me as well. We're in a bit of a time squeeze right now. I'd like to clarify the comment on top. >>> + * val' is val with ',,' replaced by ',' >>> + * and only R may be empty. >>> + * >>> + * Duplicate keys are permitted; all but the last one are ignored. >>> + * >>> + * The equations must have a solution. Counter-example: a.b=1,a=2 >>> + * doesn't have one, because R.a must be an object to satisfy a.b=1 >>> + * and a string to satisfy a=2. >>> + * >>> + * The length of any key-fragment must be between 1 and 127. >>> + * >>> + * Design flaw: there is no way to denote an empty non-root object. >>> + * While interpreting "key absent" as empty object seems natural >>> + * (removing a key-val from the input string removes the member when >>> + * there are more, so why not when it's the last), it doesn't work: >>> + * "key absent" already means "optional object absent", which isn't >>> + * the same as "empty object present". >>> + * >>> + * Additional syntax for use with an implied key: >>> + * >>> + * key-vals-ik = val-no-key [ ',' key-vals ] >>> + * val-no-key = / [^,]* / > > I think this needs to be [^,=]*, since the presence of an = means you've > supplied a key, and are not using the implied-key sugar. You're right. >>> + * >>> + * where no-key is syntactic sugar for implied-key=val-no-key. >> >> s/no-key/val-no-key/ ? >> >>> + * >>> + * TODO support lists >>> + * TODO support key-fragment with __RFQDN_ prefix (downstream extensions) >> >> Worth another TODO comment for implied values that contain a comma? The >> current restriction feels a bit artificial. > > It may be a bit artificial, but at least we can document it: implied > keys are sugar that can only be used for certain values, but you can > always avoid the sugar and explicitly provide the key=value for > problematic values that can't be done with the implied key.