Hi On Fri, Mar 11, 2022 at 8:46 PM Steven Sistare wrote: > On 3/9/2022 9:11 AM, Marc-André Lureau wrote: > > Hi > > > > On Wed, Dec 22, 2021 at 11:42 PM Steve Sistare < > steven.sistare@oracle.com > wrote: > > > > Generalize strList_from_comma_list() to take any delimiter > character, rename > > as strList_from_string(), and move it to qapi/util.c. Also add > > strv_from_strList() and QAPI_LIST_LENGTH(). > > > > Looks like you could easily split, and add some tests. > > Will do. > I don't see any tests that include qapi/util.h, so this will be a new test > file. > > For the split, how about: > patch: qapi: strList_from_string > patch: qapi: strv_from_strList > patch: qapi: QAPI_LIST_LENGTH > patch: qapi: unit tests for lists > > Sure, that's fine > Or do you prefer that unit tests be pushed with each function's patch? > I don't have a strong preference. I usually prefer new code coming with its own test, but if the resulting patch becomes too large, or if the test touches other related aspects, might be better off as different patch. Up to you! > > No functional change. > > > > Signed-off-by: Steve Sistare steven.sistare@oracle.com>> > > --- > > include/qapi/util.h | 28 ++++++++++++++++++++++++++++ > > monitor/hmp-cmds.c | 29 ++--------------------------- > > qapi/qapi-util.c | 37 +++++++++++++++++++++++++++++++++++++ > > 3 files changed, 67 insertions(+), 27 deletions(-) > > > > diff --git a/include/qapi/util.h b/include/qapi/util.h > > index 81a2b13..c249108 100644 > > --- a/include/qapi/util.h > > +++ b/include/qapi/util.h > > @@ -22,6 +22,8 @@ typedef struct QEnumLookup { > > const int size; > > } QEnumLookup; > > > > +struct strList; > > + > > const char *qapi_enum_lookup(const QEnumLookup *lookup, int val); > > int qapi_enum_parse(const QEnumLookup *lookup, const char *buf, > > int def, Error **errp); > > @@ -31,6 +33,19 @@ bool qapi_bool_parse(const char *name, const char > *value, bool *obj, > > int parse_qapi_name(const char *name, bool complete); > > > > /* > > + * Produce and return a NULL-terminated array of strings from @args. > > + * All strings are g_strdup'd. > > + */ > > +char **strv_from_strList(const struct strList *args); > > > > + > > > > I'd suggest to use the dedicated glib type GStrv > > Will do, here and in related code. > thanks > > - Steve > > > +/* > > + * Produce a strList from the character delimited string @in. > > + * All strings are g_strdup'd. > > + * A NULL or empty input string returns NULL. > > + */ > > +struct strList *strList_from_string(const char *in, char delim); > > + > > +/* > > * For any GenericList @list, insert @element at the front. > > * > > * Note that this macro evaluates @element exactly once, so it is > safe > > @@ -56,4 +71,17 @@ int parse_qapi_name(const char *name, bool > complete); > > (tail) = &(*(tail))->next; \ > > } while (0) > > > > +/* > > + * For any GenericList @list, return its length. > > + */ > > +#define QAPI_LIST_LENGTH(list) \ > > + ({ \ > > + int len = 0; \ > > + typeof(list) elem; \ > > + for (elem = list; elem != NULL; elem = elem->next) { \ > > + len++; \ > > + } \ > > + len; \ > > + }) > > + > > #endif > > diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c > > index b8c22da..5ca8b4b 100644 > > --- a/monitor/hmp-cmds.c > > +++ b/monitor/hmp-cmds.c > > @@ -43,6 +43,7 @@ > > #include "qapi/qapi-commands-run-state.h" > > #include "qapi/qapi-commands-tpm.h" > > #include "qapi/qapi-commands-ui.h" > > +#include "qapi/util.h" > > #include "qapi/qapi-visit-net.h" > > #include "qapi/qapi-visit-migration.h" > > #include "qapi/qmp/qdict.h" > > @@ -70,32 +71,6 @@ bool hmp_handle_error(Monitor *mon, Error *err) > > return false; > > } > > > > -/* > > - * Produce a strList from a comma separated list. > > - * A NULL or empty input string return NULL. > > - */ > > -static strList *strList_from_comma_list(const char *in) > > -{ > > - strList *res = NULL; > > - strList **tail = &res; > > - > > - while (in && in[0]) { > > - char *comma = strchr(in, ','); > > - char *value; > > - > > - if (comma) { > > - value = g_strndup(in, comma - in); > > - in = comma + 1; /* skip the , */ > > - } else { > > - value = g_strdup(in); > > - in = NULL; > > - } > > - QAPI_LIST_APPEND(tail, value); > > - } > > - > > - return res; > > -} > > - > > void hmp_info_name(Monitor *mon, const QDict *qdict) > > { > > NameInfo *info; > > @@ -1103,7 +1078,7 @@ void hmp_announce_self(Monitor *mon, const > QDict *qdict) > > > migrate_announce_params()); > > > > qapi_free_strList(params->interfaces); > > - params->interfaces = strList_from_comma_list(interfaces_str); > > + params->interfaces = strList_from_string(interfaces_str, ','); > > params->has_interfaces = params->interfaces != NULL; > > params->id = g_strdup(id); > > params->has_id = !!params->id; > > diff --git a/qapi/qapi-util.c b/qapi/qapi-util.c > > index fda7044..edd51b3 100644 > > --- a/qapi/qapi-util.c > > +++ b/qapi/qapi-util.c > > @@ -15,6 +15,7 @@ > > #include "qapi/error.h" > > #include "qemu/ctype.h" > > #include "qapi/qmp/qerror.h" > > +#include "qapi/qapi-builtin-types.h" > > > > CompatPolicy compat_policy; > > > > @@ -152,3 +153,39 @@ int parse_qapi_name(const char *str, bool > complete) > > } > > return p - str; > > } > > + > > +char **strv_from_strList(const strList *args) > > +{ > > + const strList *arg; > > + int i = 0; > > + char **argv = g_malloc((QAPI_LIST_LENGTH(args) + 1) * > sizeof(char *)); > > + > > + for (arg = args; arg != NULL; arg = arg->next) { > > + argv[i++] = g_strdup(arg->value); > > + } > > + argv[i] = NULL; > > + > > + return argv; > > +} > > + > > +strList *strList_from_string(const char *in, char delim) > > +{ > > + strList *res = NULL; > > + strList **tail = &res; > > + > > + while (in && in[0]) { > > + char *next = strchr(in, delim); > > + char *value; > > + > > + if (next) { > > + value = g_strndup(in, next - in); > > + in = next + 1; /* skip the delim */ > > + } else { > > + value = g_strdup(in); > > + in = NULL; > > + } > > + QAPI_LIST_APPEND(tail, value); > > + } > > + > > + return res; > > +} > > -- > > 1.8.3.1 > > > > > > > > > > -- > > Marc-André Lureau > -- Marc-André Lureau