Am 28.02.2017 um 23:11 hat Markus Armbruster geschrieben: > Kevin Wolf <kw...@redhat.com> writes: > > > Am 28.02.2017 um 22:27 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]. > >> > >> Examples: > >> > >> * list.1=goner,list.0=null,list.1=eins,list.2=zwei > >> is equivalent to JSON [ "null", "eins", "zwei" ] > >> > >> * a.b.c=1,a.b.0=2 > >> is inconsistent: a.b.c clashes with a.b.0 > >> > >> * list.0=null,list.2=eins,list.2=zwei > >> has a hole: list.1 is missing > >> > >> Similar design flaw as for objects: there is no way to denote an empty > >> list. While interpreting "key absent" as empty list seems natural > >> (removing a list member from the input string works when there are > >> multiple ones, so why not when there's just one), it doesn't work: > >> "key absent" already means "optional list absent", which isn't the > >> same as "empty list present". > >> > >> Update the keyval object visitor to use this a.0 syntax in error > >> messages rather than the usual a[0]. > >> > >> Signed-off-by: Markus Armbruster <arm...@redhat.com> > > > >> +/* > >> + * Listify @cur recursively. > >> + * Replace QDicts whose keys are all valid list indexes by QLists. > >> + * @key_of_cur is the list of key fragments leading up to @cur. > >> + * On success, return either @cur or its replacement. > >> + * On failure, store an error through @errp and return NULL. > >> + */ > >> +static QObject *keyval_listify(QDict *cur, GSList *key_of_cur, Error > >> **errp) > >> +{ > >> + GSList key_node; > >> + bool has_index, has_member; > >> + const QDictEntry *ent; > >> + QDict *qdict; > >> + QObject *val; > >> + char *key; > >> + size_t nelt; > >> + QObject **elt; > >> + int index, max_index, i; > >> + QList *list; > >> + > >> + key_node.next = key_of_cur; > >> + > >> + /* > >> + * Recursively listify @cur's members, and figure out whether @cur > >> + * itself is to be listified. > >> + */ > >> + has_index = false; > >> + has_member = false; > >> + for (ent = qdict_first(cur); ent; ent = qdict_next(cur, ent)) { > >> + if (key_to_index(ent->key, NULL) >= 0) { > >> + has_index = true; > >> + } else { > >> + has_member = true; > >> + } > >> + > >> + qdict = qobject_to_qdict(ent->value); > >> + if (!qdict) { > >> + continue; > >> + } > >> + > >> + key_node.data = ent->key; > >> + val = keyval_listify(qdict, &key_node, errp); > >> + if (!val) { > >> + return NULL; > >> + } > >> + if (val != ent->value) { > >> + qdict_put_obj(cur, ent->key, val); > >> + } > >> + } > >> + > >> + if (has_index && has_member) { > >> + key = reassemble_key(key_of_cur); > >> + error_setg(errp, "Parameters '%s*' used inconsistently", key); > >> + g_free(key); > >> + return NULL; > >> + } > >> + if (!has_index) { > >> + return QOBJECT(cur); > >> + } > >> + > >> + /* Copy @cur's values to @elt[] */ > >> + nelt = qdict_size(cur) + 1; /* one extra, for use as sentinel */ > >> + elt = g_new0(QObject *, nelt); > >> + max_index = -1; > >> + for (ent = qdict_first(cur); ent; ent = qdict_next(cur, ent)) { > >> + index = key_to_index(ent->key, NULL); > >> + assert(index >= 0); > >> + if (index > max_index) { > >> + max_index = index; > >> + } > >> + /* > >> + * We iterate @nelt times. If we get one exceeding @nelt > >> + * here, we will put less than @nelt values into @elt[], > >> + * triggering the error in the next loop. > >> + */ > >> + if ((size_t)index >= nelt) { > > > > I think this needs to be index >= nelt - 1 now... > > > >> + continue; > >> + } > >> + /* Even though dict keys are distinct, indexes need not be */ > >> + elt[index] = ent->value; > >> + } > >> + > >> + /* > >> + * Make a list from @elt[], reporting any missing elements. > >> + * If we dropped an index >= nelt in the previous loop, this loop > >> + * will run into the sentinel and report index @nelt missing. > >> + */ > >> + list = qlist_new(); > >> + assert(!elt[nelt-1]); /* need the sentinel to be null */ > > > > ...or this assertion can fail. > > Yup. > > Appended fix squashed in and pushed to my public repo. R-by? > > >> + for (i = 0; i < MIN(nelt, max_index + 1); i++) { > >> + if (!elt[i]) { > >> + key = reassemble_key(key_of_cur); > >> + error_setg(errp, "Parameter '%s%d' missing", key, i); > >> + g_free(key); > >> + g_free(elt); > >> + QDECREF(list); > >> + return NULL; > >> + } > >> + qobject_incref(elt[i]); > >> + qlist_append_obj(list, elt[i]); > >> + } > >> + > >> + g_free(elt); > >> + return QOBJECT(list); > >> +} > > > > Kevin > > > diff --git a/util/keyval.c b/util/keyval.c > index b57a742..c316aaa 100644 > --- a/util/keyval.c > +++ b/util/keyval.c > @@ -329,7 +329,7 @@ static QObject *keyval_listify(QDict *cur, GSList > *key_of_cur, Error **errp) > * here, we will put less than @nelt values into @elt[], > * triggering the error in the next loop. > */ > - if ((size_t)index >= nelt) { > + if ((size_t)index >= nelt - 1) { > continue; > } > /* Even though dict keys are distinct, indexes need not be */
With this squashed in: Reviewed-by: Kevin Wolf <kw...@redhat.com>