| Message ID | 1640199934-455149-12-git-send-email-steven.sistare@oracle.com |
|---|---|
| State | New |
| Headers | show |
| Series | Live Update | expand |
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. > > 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 > +/* > + * 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 > > >
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 <mailto: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 Or do you prefer that unit tests be pushed with each function's patch? > No functional change. > > Signed-off-by: Steve Sistare <steven.sistare@oracle.com <mailto: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. - 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
Hi On Fri, Mar 11, 2022 at 8:46 PM Steven Sistare <steven.sistare@oracle.com> 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 <mailto: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 <mailto: > 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 >
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); + +/* + * 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; +}
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(). 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(-)