On 02/28/2017 01:25 PM, Kevin Wolf wrote: > Am 27.02.2017 um 12:20 hat Markus Armbruster geschrieben: >> Additionally permit non-negative integers as key components. A >> dictionary's keys must either be all integers or none. If all keys >> are integers, convert the dictionary to a list. The set of keys must >> be [0,N]. >>
>> @@ -34,16 +36,16 @@ >> * doesn't have one, because R.a must be an object to satisfy a.b=1 >> * and a string to satisfy a=2. >> * >> - * Key-fragments must be valid QAPI names. >> + * Key-fragments must be valid QAPI names or consist only of digits. >> /* >> + * Convert @key to a list index. >> + * Convert all leading digits to a (non-negative) number, capped at >> + * INT_MAX. >> + * If @end is non-null, assign a pointer to the first character after >> + * the number to *@end. >> + * Else, fail if any characters follow. >> + * On success, return the converted number. >> + * On failure, return a negative value. >> + * Note: since only digits are converted, no two keys can map to the >> + * same number, except by overflow to INT_MAX. >> + */ >> +static int key_to_index(const char *key, const char **end) >> +{ >> + int ret; >> + unsigned long index; >> + >> + if (*key < '0' || *key > '9') { >> + return -EINVAL; >> + } So no leading whitespace, '+', or '-', even if strtoul would have allowed it. Such names are also invalid as member id names. (There's still the question if we want to allow arbitrary whitespace after between-key-value ',', and maybe even after between-key-segment '.' after this series, to make it easier to write strategically line-wrapped command lines - but that's an independent thing to be done on top). >> @@ -137,8 +165,13 @@ static const char *keyval_parse_one(QDict *qdict, const >> char *params, >> cur = qdict; >> s = key; >> for (;;) { >> - ret = parse_qapi_name(s, false); >> - len = ret < 0 ? 0 : ret; >> + /* Want a key index (unless it's first) or a QAPI name */ >> + if (s != key && key_to_index(s, &end) >= 0) { >> + len = end - s; >> + } else { >> + ret = parse_qapi_name(s, false); >> + len = ret < 0 ? 0 : ret; >> + } Does this mishandle keyval_parse(string, "0", err) - where we want to assert that the caller always passes only a valid id name for an implicit key? >> assert(s + len <= key_end); >> if (!len || (s + len < key_end && s[len] != '.')) { >> assert(key != implied_key); >> @@ -205,6 +238,119 @@ static const char *keyval_parse_one(QDict *qdict, >> const char *params, >> return s; >> } >> >> +static char *reassemble_key(GSList *key) >> +{ >> + GString *s = g_string_new(""); >> + GSList *p; >> + >> + for (p = key; p; p = p->next) { >> + g_string_prepend_c(s, '.'); >> + g_string_prepend(s, (char *)p->data); Should this use the canonical form of an index, even if the user spelled it with extra bytes like leading zero? >> + /* Copy @cur's values to @elt[] */ >> + nelt = qdict_size(cur); >> + elt = g_new0(QObject *, nelt); > > This doesn't seem to be freed. > >> + for (ent = qdict_first(cur); ent; ent = qdict_next(cur, ent)) { >> + index = key_to_index(ent->key, NULL); >> + assert(index >= 0); >> + /* >> + * We iterate @nelt times. Because the dictionary keys are >> + * distinct, the indexes are also distinct (key_to_index() >> + * ensures it). > > Really? What about leading zeros? key_to_index() should be used to convert "01" and "1" into the same key when first computing the QDict during the initial parse; this post-pass should thus only see a single "1" key (last-one-wins semantics, regardless of the difference in spelling of the same index repeated on the command line). -- Eric Blake eblake redhat com +1-919-301-3266 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature