qemu-devel.nongnu.org archive mirror
 help / color / mirror / Atom feed
* [PATCH V5 0/5] string list functions
@ 2024-02-22 21:47 Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
                   ` (4 more replies)
  0 siblings, 5 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Add some handy string list functions for general use, and use them in
live migration functions.  These will also be needed for cpr exec mode.

Changes in V5:
  * renamed some variables and one function, replaced GStrv with char **
  * aligned backslashes in QAPI_LIST_LENGTH
  * restored cutils.h to exec.c

Changes in V4:
  * added exec migration patch

Steve Sistare (5):
  util: str_split
  qapi: QAPI_LIST_LENGTH
  util: strv_from_strList
  util: strList unit tests
  migration: simplify exec migration functions

 include/monitor/hmp.h     |  1 -
 include/qapi/util.h       | 13 ++++++++
 include/qemu/strList.h    | 30 ++++++++++++++++++
 migration/exec.c          | 57 +++++----------------------------
 monitor/hmp-cmds.c        | 19 -----------
 net/net-hmp-cmds.c        |  3 +-
 stats/stats-hmp-cmds.c    |  3 +-
 tests/unit/meson.build    |  1 +
 tests/unit/test-strList.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 util/meson.build          |  1 +
 util/strList.c            | 38 ++++++++++++++++++++++
 11 files changed, 175 insertions(+), 71 deletions(-)
 create mode 100644 include/qemu/strList.h
 create mode 100644 tests/unit/test-strList.c
 create mode 100644 util/strList.c

-- 
1.8.3.1



^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH V5 1/5] util: str_split
  2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
@ 2024-02-22 21:47 ` Steve Sistare
  2024-02-23  5:51   ` Philippe Mathieu-Daudé
                     ` (2 more replies)
  2024-02-22 21:47 ` [PATCH V5 2/5] qapi: QAPI_LIST_LENGTH Steve Sistare
                   ` (3 subsequent siblings)
  4 siblings, 3 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Generalize hmp_split_at_comma() to take any delimiter string, rename
as str_split(), and move it to util/strList.c.

No functional change.

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
---
 include/monitor/hmp.h  |  1 -
 include/qemu/strList.h | 24 ++++++++++++++++++++++++
 monitor/hmp-cmds.c     | 19 -------------------
 net/net-hmp-cmds.c     |  3 ++-
 stats/stats-hmp-cmds.c |  3 ++-
 util/meson.build       |  1 +
 util/strList.c         | 24 ++++++++++++++++++++++++
 7 files changed, 53 insertions(+), 22 deletions(-)
 create mode 100644 include/qemu/strList.h
 create mode 100644 util/strList.c

diff --git a/include/monitor/hmp.h b/include/monitor/hmp.h
index 13f9a2d..2df661e 100644
--- a/include/monitor/hmp.h
+++ b/include/monitor/hmp.h
@@ -19,7 +19,6 @@
 
 bool hmp_handle_error(Monitor *mon, Error *err);
 void hmp_help_cmd(Monitor *mon, const char *name);
-strList *hmp_split_at_comma(const char *str);
 
 void hmp_info_name(Monitor *mon, const QDict *qdict);
 void hmp_info_version(Monitor *mon, const QDict *qdict);
diff --git a/include/qemu/strList.h b/include/qemu/strList.h
new file mode 100644
index 0000000..0f26116
--- /dev/null
+++ b/include/qemu/strList.h
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2022 - 2024 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#ifndef QEMU_STR_LIST_H
+#define QEMU_STR_LIST_H
+
+#include "qapi/qapi-builtin-types.h"
+
+/*
+ * Split @str into a strList using the delimiter string @delim.
+ * The delimiter is not included in the result.
+ * Return NULL if @str is NULL or an empty string.
+ * A leading, trailing, or consecutive delimiter produces an
+ * empty string at that position in the output.
+ * All strings are g_strdup'd, and the result can be freed
+ * using qapi_free_strList.
+ */
+strList *str_split(const char *str, const char *delim);
+
+#endif
diff --git a/monitor/hmp-cmds.c b/monitor/hmp-cmds.c
index 871898a..66b68a0 100644
--- a/monitor/hmp-cmds.c
+++ b/monitor/hmp-cmds.c
@@ -38,25 +38,6 @@ bool hmp_handle_error(Monitor *mon, Error *err)
     return false;
 }
 
-/*
- * Split @str at comma.
- * A null @str defaults to "".
- */
-strList *hmp_split_at_comma(const char *str)
-{
-    char **split = g_strsplit(str ?: "", ",", -1);
-    strList *res = NULL;
-    strList **tail = &res;
-    int i;
-
-    for (i = 0; split[i]; i++) {
-        QAPI_LIST_APPEND(tail, split[i]);
-    }
-
-    g_free(split);
-    return res;
-}
-
 void hmp_info_name(Monitor *mon, const QDict *qdict)
 {
     NameInfo *info;
diff --git a/net/net-hmp-cmds.c b/net/net-hmp-cmds.c
index 41d326b..969cdd1 100644
--- a/net/net-hmp-cmds.c
+++ b/net/net-hmp-cmds.c
@@ -26,6 +26,7 @@
 #include "qemu/config-file.h"
 #include "qemu/help_option.h"
 #include "qemu/option.h"
+#include "qemu/strList.h"
 
 void hmp_info_network(Monitor *mon, const QDict *qdict)
 {
@@ -72,7 +73,7 @@ void hmp_announce_self(Monitor *mon, const QDict *qdict)
                                             migrate_announce_params());
 
     qapi_free_strList(params->interfaces);
-    params->interfaces = hmp_split_at_comma(interfaces_str);
+    params->interfaces = str_split(interfaces_str, ",");
     params->has_interfaces = params->interfaces != NULL;
     params->id = g_strdup(id);
     qmp_announce_self(params, NULL);
diff --git a/stats/stats-hmp-cmds.c b/stats/stats-hmp-cmds.c
index 1f91bf8..62db8c6 100644
--- a/stats/stats-hmp-cmds.c
+++ b/stats/stats-hmp-cmds.c
@@ -10,6 +10,7 @@
 #include "monitor/hmp.h"
 #include "monitor/monitor.h"
 #include "qemu/cutils.h"
+#include "qemu/strList.h"
 #include "hw/core/cpu.h"
 #include "qapi/qmp/qdict.h"
 #include "qapi/error.h"
@@ -176,7 +177,7 @@ static StatsFilter *stats_filter(StatsTarget target, const char *names,
             request->provider = provider_idx;
             if (names && !g_str_equal(names, "*")) {
                 request->has_names = true;
-                request->names = hmp_split_at_comma(names);
+                request->names = str_split(names, ",");
             }
             QAPI_LIST_PREPEND(request_list, request);
         }
diff --git a/util/meson.build b/util/meson.build
index 0ef9886..bd125a4 100644
--- a/util/meson.build
+++ b/util/meson.build
@@ -1,4 +1,5 @@
 util_ss.add(files('osdep.c', 'cutils.c', 'unicode.c', 'qemu-timer-common.c'))
+util_ss.add(files('strList.c'))
 util_ss.add(files('thread-context.c'), numa)
 if not config_host_data.get('CONFIG_ATOMIC64')
   util_ss.add(files('atomic64.c'))
diff --git a/util/strList.c b/util/strList.c
new file mode 100644
index 0000000..7588c7c
--- /dev/null
+++ b/util/strList.c
@@ -0,0 +1,24 @@
+/*
+ * Copyright (c) 2023 Red Hat, Inc.
+ * Copyright (c) 2022 - 2024 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/strList.h"
+
+strList *str_split(const char *str, const char *delim)
+{
+    g_autofree char **split = g_strsplit(str ?: "", delim, -1);
+    strList *res = NULL;
+    strList **tail = &res;
+    int i;
+
+    for (i = 0; split[i]; i++) {
+        QAPI_LIST_APPEND(tail, split[i]);
+    }
+
+    return res;
+}
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V5 2/5] qapi: QAPI_LIST_LENGTH
  2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
@ 2024-02-22 21:47 ` Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 3/5] util: strv_from_strList Steve Sistare
                   ` (2 subsequent siblings)
  4 siblings, 0 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qapi/util.h | 13 +++++++++++++
 1 file changed, 13 insertions(+)

diff --git a/include/qapi/util.h b/include/qapi/util.h
index 81a2b13..20dfea8 100644
--- a/include/qapi/util.h
+++ b/include/qapi/util.h
@@ -56,4 +56,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)                                      \
+    ({                                                              \
+        size_t _len = 0;                                            \
+        typeof(list) _tail;                                         \
+        for (_tail = list; _tail != NULL; _tail = _tail->next) {    \
+            _len++;                                                 \
+        }                                                           \
+        _len;                                                       \
+    })
+
 #endif
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V5 3/5] util: strv_from_strList
  2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 2/5] qapi: QAPI_LIST_LENGTH Steve Sistare
@ 2024-02-22 21:47 ` Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 4/5] util: strList unit tests Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 5/5] migration: simplify exec migration functions Steve Sistare
  4 siblings, 0 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 include/qemu/strList.h |  6 ++++++
 util/strList.c         | 14 ++++++++++++++
 2 files changed, 20 insertions(+)

diff --git a/include/qemu/strList.h b/include/qemu/strList.h
index 0f26116..c1eb1dd 100644
--- a/include/qemu/strList.h
+++ b/include/qemu/strList.h
@@ -21,4 +21,10 @@
  */
 strList *str_split(const char *str, const char *delim);
 
+/*
+ * Produce and return a NULL-terminated array of strings from @list.
+ * The result is g_malloc'd and all strings are g_strdup'd.
+ */
+char **strv_from_strList(const strList *list);
+
 #endif
diff --git a/util/strList.c b/util/strList.c
index 7588c7c..6da6762 100644
--- a/util/strList.c
+++ b/util/strList.c
@@ -22,3 +22,17 @@ strList *str_split(const char *str, const char *delim)
 
     return res;
 }
+
+char **strv_from_strList(const strList *list)
+{
+    const strList *tail;
+    int i = 0;
+    char **argv = g_new(char *, QAPI_LIST_LENGTH(list) + 1);
+
+    for (tail = list; tail != NULL; tail = tail->next) {
+        argv[i++] = g_strdup(tail->value);
+    }
+    argv[i] = NULL;
+
+    return argv;
+}
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V5 4/5] util: strList unit tests
  2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
                   ` (2 preceding siblings ...)
  2024-02-22 21:47 ` [PATCH V5 3/5] util: strv_from_strList Steve Sistare
@ 2024-02-22 21:47 ` Steve Sistare
  2024-02-22 21:47 ` [PATCH V5 5/5] migration: simplify exec migration functions Steve Sistare
  4 siblings, 0 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Marc-André Lureau <marcandre.lureau@redhat.com>
---
 tests/unit/meson.build    |  1 +
 tests/unit/test-strList.c | 80 +++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 81 insertions(+)
 create mode 100644 tests/unit/test-strList.c

diff --git a/tests/unit/meson.build b/tests/unit/meson.build
index cae925c..9984860 100644
--- a/tests/unit/meson.build
+++ b/tests/unit/meson.build
@@ -35,6 +35,7 @@ tests = {
   'test-rcu-simpleq': [],
   'test-rcu-tailq': [],
   'test-rcu-slist': [],
+  'test-strList': [],
   'test-qdist': [],
   'test-qht': [],
   'test-qtree': [],
diff --git a/tests/unit/test-strList.c b/tests/unit/test-strList.c
new file mode 100644
index 0000000..40af6b2
--- /dev/null
+++ b/tests/unit/test-strList.c
@@ -0,0 +1,80 @@
+/*
+ * Copyright (c) 2022 - 2024 Oracle and/or its affiliates.
+ *
+ * This work is licensed under the terms of the GNU GPL, version 2 or later.
+ * See the COPYING file in the top-level directory.
+ */
+
+#include "qemu/osdep.h"
+#include "qemu/strList.h"
+
+static strList *make_list(int length)
+{
+    strList *head = 0, *list, **prev = &head;
+
+    while (length--) {
+        list = *prev = g_new0(strList, 1);
+        list->value = g_strdup("aaa");
+        prev = &list->next;
+    }
+    return head;
+}
+
+static void test_length(void)
+{
+    strList *list;
+    int i;
+
+    for (i = 0; i < 5; i++) {
+        list = make_list(i);
+        g_assert_cmpint(i, ==, QAPI_LIST_LENGTH(list));
+        qapi_free_strList(list);
+    }
+}
+
+struct {
+    const char *string;
+    const char *delim;
+    const char *argv[5];
+} list_data[] = {
+    { NULL, ",", { NULL } },
+    { "", ",", { NULL } },
+    { "a", ",", { "a", NULL } },
+    { "a,b", ",", { "a", "b", NULL } },
+    { "a,b,c", ",", { "a", "b", "c", NULL } },
+    { "first last", " ", { "first", "last", NULL } },
+    { "a:", ":", { "a", "", NULL } },
+    { "a::b", ":", { "a", "", "b", NULL } },
+    { ":", ":", { "", "", NULL } },
+    { ":a", ":", { "", "a", NULL } },
+    { "::a", ":", { "", "", "a", NULL } },
+};
+
+static void test_strv(void)
+{
+    int i, j;
+    const char **expect;
+    strList *list;
+    char **argv;
+
+    for (i = 0; i < ARRAY_SIZE(list_data); i++) {
+        expect = list_data[i].argv;
+        list = str_split(list_data[i].string, list_data[i].delim);
+        argv = strv_from_strList(list);
+        qapi_free_strList(list);
+        for (j = 0; expect[j] && argv[j]; j++) {
+            g_assert_cmpstr(expect[j], ==, argv[j]);
+        }
+        g_assert_null(expect[j]);
+        g_assert_null(argv[j]);
+        g_strfreev(argv);
+    }
+}
+
+int main(int argc, char **argv)
+{
+    g_test_init(&argc, &argv, NULL);
+    g_test_add_func("/test-string/length", test_length);
+    g_test_add_func("/test-string/strv", test_strv);
+    return g_test_run();
+}
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* [PATCH V5 5/5] migration: simplify exec migration functions
  2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
                   ` (3 preceding siblings ...)
  2024-02-22 21:47 ` [PATCH V5 4/5] util: strList unit tests Steve Sistare
@ 2024-02-22 21:47 ` Steve Sistare
  4 siblings, 0 replies; 12+ messages in thread
From: Steve Sistare @ 2024-02-22 21:47 UTC (permalink / raw)
  To: qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau, Steve Sistare

Simplify the exec migration code by using list utility functions.

As a side effect, this also fixes a minor memory leak.  On function return,
"g_auto(GStrv) argv" frees argv and each element, which is wrong, because
the function does not own the individual elements.  To compensate, the code
uses g_steal_pointer which NULLs argv and prevents the destructor from
running, but argv is leaked.

Fixes: cbab4face57b ("migration: convert exec backend ...")
Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
Reviewed-by: Fabiano Rosas <farosas@suse.de>
---
 migration/exec.c | 57 ++++++++------------------------------------------------
 1 file changed, 8 insertions(+), 49 deletions(-)

diff --git a/migration/exec.c b/migration/exec.c
index 47d2f3b..1518409 100644
--- a/migration/exec.c
+++ b/migration/exec.c
@@ -19,6 +19,7 @@
 
 #include "qemu/osdep.h"
 #include "qemu/error-report.h"
+#include "qemu/strList.h"
 #include "channel.h"
 #include "exec.h"
 #include "migration.h"
@@ -39,51 +40,16 @@ const char *exec_get_cmd_path(void)
 }
 #endif
 
-/* provides the length of strList */
-static int
-str_list_length(strList *list)
-{
-    int len = 0;
-    strList *elem;
-
-    for (elem = list; elem != NULL; elem = elem->next) {
-        len++;
-    }
-
-    return len;
-}
-
-static void
-init_exec_array(strList *command, char **argv, Error **errp)
-{
-    int i = 0;
-    strList *lst;
-
-    for (lst = command; lst; lst = lst->next) {
-        argv[i++] = lst->value;
-    }
-
-    argv[i] = NULL;
-    return;
-}
-
 void exec_start_outgoing_migration(MigrationState *s, strList *command,
                                    Error **errp)
 {
-    QIOChannel *ioc;
-
-    int length = str_list_length(command);
-    g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
-
-    init_exec_array(command, argv, errp);
+    QIOChannel *ioc = NULL;
+    g_auto(GStrv) argv = strv_from_strList(command);
+    const char * const *args = (const char * const *) argv;
     g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
 
     trace_migration_exec_outgoing(new_command);
-    ioc = QIO_CHANNEL(
-        qio_channel_command_new_spawn(
-                            (const char * const *) g_steal_pointer(&argv),
-                            O_RDWR,
-                            errp));
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
     if (!ioc) {
         return;
     }
@@ -105,19 +71,12 @@ static gboolean exec_accept_incoming_migration(QIOChannel *ioc,
 void exec_start_incoming_migration(strList *command, Error **errp)
 {
     QIOChannel *ioc;
-
-    int length = str_list_length(command);
-    g_auto(GStrv) argv = (char **) g_new0(const char *, length + 1);
-
-    init_exec_array(command, argv, errp);
+    g_auto(GStrv) argv = strv_from_strList(command);
+    const char * const *args = (const char * const *) argv;
     g_autofree char *new_command = g_strjoinv(" ", (char **)argv);
 
     trace_migration_exec_incoming(new_command);
-    ioc = QIO_CHANNEL(
-        qio_channel_command_new_spawn(
-                            (const char * const *) g_steal_pointer(&argv),
-                            O_RDWR,
-                            errp));
+    ioc = QIO_CHANNEL(qio_channel_command_new_spawn(args, O_RDWR, errp));
     if (!ioc) {
         return;
     }
-- 
1.8.3.1



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
@ 2024-02-23  5:51   ` Philippe Mathieu-Daudé
  2024-02-23  6:01   ` Philippe Mathieu-Daudé
  2024-02-26 12:21   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-23  5:51 UTC (permalink / raw)
  To: Steve Sistare, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 22/2/24 22:47, Steve Sistare wrote:
> Generalize hmp_split_at_comma() to take any delimiter string, rename
> as str_split(), and move it to util/strList.c.
> 
> No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   include/monitor/hmp.h  |  1 -
>   include/qemu/strList.h | 24 ++++++++++++++++++++++++
>   monitor/hmp-cmds.c     | 19 -------------------
>   net/net-hmp-cmds.c     |  3 ++-
>   stats/stats-hmp-cmds.c |  3 ++-
>   util/meson.build       |  1 +
>   util/strList.c         | 24 ++++++++++++++++++++++++
>   7 files changed, 53 insertions(+), 22 deletions(-)
>   create mode 100644 include/qemu/strList.h
>   create mode 100644 util/strList.c

Reviewed-by: Philippe Mathieu-Daudé <philmd@linaro.org>



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
  2024-02-23  5:51   ` Philippe Mathieu-Daudé
@ 2024-02-23  6:01   ` Philippe Mathieu-Daudé
  2024-02-23 14:01     ` Steven Sistare
  2024-02-26 12:21   ` Philippe Mathieu-Daudé
  2 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-23  6:01 UTC (permalink / raw)
  To: Steve Sistare, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 22/2/24 22:47, Steve Sistare wrote:
> Generalize hmp_split_at_comma() to take any delimiter string, rename
> as str_split(), and move it to util/strList.c.
> 
> No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   include/monitor/hmp.h  |  1 -
>   include/qemu/strList.h | 24 ++++++++++++++++++++++++
>   monitor/hmp-cmds.c     | 19 -------------------
>   net/net-hmp-cmds.c     |  3 ++-
>   stats/stats-hmp-cmds.c |  3 ++-
>   util/meson.build       |  1 +
>   util/strList.c         | 24 ++++++++++++++++++++++++
>   7 files changed, 53 insertions(+), 22 deletions(-)
>   create mode 100644 include/qemu/strList.h
>   create mode 100644 util/strList.c


> +#include "qapi/qapi-builtin-types.h"
> +
> +/*
> + * Split @str into a strList using the delimiter string @delim.
> + * The delimiter is not included in the result.
> + * Return NULL if @str is NULL or an empty string.
> + * A leading, trailing, or consecutive delimiter produces an
> + * empty string at that position in the output.
> + * All strings are g_strdup'd, and the result can be freed
> + * using qapi_free_strList.

Note "qapi/qapi-builtin-types.h" defines:

   G_DEFINE_AUTOPTR_CLEANUP_FUNC(strList, qapi_free_strList)

Maybe mention we can also use:

   g_autoptr(strList)

?

> + */
> +strList *str_split(const char *str, const char *delim);
> +
> +#endif



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-23  6:01   ` Philippe Mathieu-Daudé
@ 2024-02-23 14:01     ` Steven Sistare
  2024-02-23 17:41       ` Philippe Mathieu-Daudé
  0 siblings, 1 reply; 12+ messages in thread
From: Steven Sistare @ 2024-02-23 14:01 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 2/23/2024 1:01 AM, Philippe Mathieu-Daudé wrote:
> On 22/2/24 22:47, Steve Sistare wrote:
>> Generalize hmp_split_at_comma() to take any delimiter string, rename
>> as str_split(), and move it to util/strList.c.
>>
>> No functional change.
>>
>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>> ---
>>   include/monitor/hmp.h  |  1 -
>>   include/qemu/strList.h | 24 ++++++++++++++++++++++++
>>   monitor/hmp-cmds.c     | 19 -------------------
>>   net/net-hmp-cmds.c     |  3 ++-
>>   stats/stats-hmp-cmds.c |  3 ++-
>>   util/meson.build       |  1 +
>>   util/strList.c         | 24 ++++++++++++++++++++++++
>>   7 files changed, 53 insertions(+), 22 deletions(-)
>>   create mode 100644 include/qemu/strList.h
>>   create mode 100644 util/strList.c
> 
> 
>> +#include "qapi/qapi-builtin-types.h"
>> +
>> +/*
>> + * Split @str into a strList using the delimiter string @delim.
>> + * The delimiter is not included in the result.
>> + * Return NULL if @str is NULL or an empty string.
>> + * A leading, trailing, or consecutive delimiter produces an
>> + * empty string at that position in the output.
>> + * All strings are g_strdup'd, and the result can be freed
>> + * using qapi_free_strList.
> 
> Note "qapi/qapi-builtin-types.h" defines:
> 
>   G_DEFINE_AUTOPTR_CLEANUP_FUNC(strList, qapi_free_strList)
> 
> Maybe mention we can also use:
> 
>   g_autoptr(strList)

Thanks Philippe.  If we get to V6 for this series, I will mention this,
and also mention g_autoptr(GStrv) in the header comment for strv_from_strlist.

- Steve

>> + */
>> +strList *str_split(const char *str, const char *delim);
>> +
>> +#endif
> 


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-23 14:01     ` Steven Sistare
@ 2024-02-23 17:41       ` Philippe Mathieu-Daudé
  2024-02-23 18:04         ` Steven Sistare
  0 siblings, 1 reply; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-23 17:41 UTC (permalink / raw)
  To: Steven Sistare, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 23/2/24 15:01, Steven Sistare wrote:
> On 2/23/2024 1:01 AM, Philippe Mathieu-Daudé wrote:
>> On 22/2/24 22:47, Steve Sistare wrote:
>>> Generalize hmp_split_at_comma() to take any delimiter string, rename
>>> as str_split(), and move it to util/strList.c.
>>>
>>> No functional change.
>>>
>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>> ---
>>>    include/monitor/hmp.h  |  1 -
>>>    include/qemu/strList.h | 24 ++++++++++++++++++++++++
>>>    monitor/hmp-cmds.c     | 19 -------------------
>>>    net/net-hmp-cmds.c     |  3 ++-
>>>    stats/stats-hmp-cmds.c |  3 ++-
>>>    util/meson.build       |  1 +
>>>    util/strList.c         | 24 ++++++++++++++++++++++++
>>>    7 files changed, 53 insertions(+), 22 deletions(-)
>>>    create mode 100644 include/qemu/strList.h
>>>    create mode 100644 util/strList.c
>>
>>
>>> +#include "qapi/qapi-builtin-types.h"
>>> +
>>> +/*
>>> + * Split @str into a strList using the delimiter string @delim.
>>> + * The delimiter is not included in the result.
>>> + * Return NULL if @str is NULL or an empty string.
>>> + * A leading, trailing, or consecutive delimiter produces an
>>> + * empty string at that position in the output.
>>> + * All strings are g_strdup'd, and the result can be freed
>>> + * using qapi_free_strList.
>>
>> Note "qapi/qapi-builtin-types.h" defines:
>>
>>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(strList, qapi_free_strList)
>>
>> Maybe mention we can also use:
>>
>>    g_autoptr(strList)
> 
> Thanks Philippe.  If we get to V6 for this series, I will mention this,
> and also mention g_autoptr(GStrv) in the header comment for strv_from_strlist.

If there is no need for v6, do you mind sharing here what would be
the resulting comment? Maybe Markus can update it directly...
(assuming he takes your series).



^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-23 17:41       ` Philippe Mathieu-Daudé
@ 2024-02-23 18:04         ` Steven Sistare
  0 siblings, 0 replies; 12+ messages in thread
From: Steven Sistare @ 2024-02-23 18:04 UTC (permalink / raw)
  To: Philippe Mathieu-Daudé, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 2/23/2024 12:41 PM, Philippe Mathieu-Daudé wrote:
> On 23/2/24 15:01, Steven Sistare wrote:
>> On 2/23/2024 1:01 AM, Philippe Mathieu-Daudé wrote:
>>> On 22/2/24 22:47, Steve Sistare wrote:
>>>> Generalize hmp_split_at_comma() to take any delimiter string, rename
>>>> as str_split(), and move it to util/strList.c.
>>>>
>>>> No functional change.
>>>>
>>>> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
>>>> ---
>>>>    include/monitor/hmp.h  |  1 -
>>>>    include/qemu/strList.h | 24 ++++++++++++++++++++++++
>>>>    monitor/hmp-cmds.c     | 19 -------------------
>>>>    net/net-hmp-cmds.c     |  3 ++-
>>>>    stats/stats-hmp-cmds.c |  3 ++-
>>>>    util/meson.build       |  1 +
>>>>    util/strList.c         | 24 ++++++++++++++++++++++++
>>>>    7 files changed, 53 insertions(+), 22 deletions(-)
>>>>    create mode 100644 include/qemu/strList.h
>>>>    create mode 100644 util/strList.c
>>>
>>>
>>>> +#include "qapi/qapi-builtin-types.h"
>>>> +
>>>> +/*
>>>> + * Split @str into a strList using the delimiter string @delim.
>>>> + * The delimiter is not included in the result.
>>>> + * Return NULL if @str is NULL or an empty string.
>>>> + * A leading, trailing, or consecutive delimiter produces an
>>>> + * empty string at that position in the output.
>>>> + * All strings are g_strdup'd, and the result can be freed
>>>> + * using qapi_free_strList.
>>>
>>> Note "qapi/qapi-builtin-types.h" defines:
>>>
>>>    G_DEFINE_AUTOPTR_CLEANUP_FUNC(strList, qapi_free_strList)
>>>
>>> Maybe mention we can also use:
>>>
>>>    g_autoptr(strList)
>>
>> Thanks Philippe.  If we get to V6 for this series, I will mention this,
>> and also mention g_autoptr(GStrv) in the header comment for strv_from_strlist.
> 
> If there is no need for v6, do you mind sharing here what would be
> the resulting comment? Maybe Markus can update it directly...
> (assuming he takes your series).

Sure - steve

--------------------
diff --git a/include/qemu/strList.h b/include/qemu/strList.h
index c1eb1dd..b13bd53 100644
--- a/include/qemu/strList.h
+++ b/include/qemu/strList.h
@@ -17,13 +17,16 @@
  * A leading, trailing, or consecutive delimiter produces an
  * empty string at that position in the output.
  * All strings are g_strdup'd, and the result can be freed
- * using qapi_free_strList.
+ * using qapi_free_strList, or by declaring a local variable
+ * with g_autoptr(strList).
  */
 strList *str_split(const char *str, const char *delim);

 /*
  * Produce and return a NULL-terminated array of strings from @list.
- * The result is g_malloc'd and all strings are g_strdup'd.
+ * The result is g_malloc'd and all strings are g_strdup'd.  The result
+ * can be freed using g_strfreev, or by declaring a local variable with
+ * g_auto(GStrv).
  */
 char **strv_from_strList(const strList *list);

--------------------


^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH V5 1/5] util: str_split
  2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
  2024-02-23  5:51   ` Philippe Mathieu-Daudé
  2024-02-23  6:01   ` Philippe Mathieu-Daudé
@ 2024-02-26 12:21   ` Philippe Mathieu-Daudé
  2 siblings, 0 replies; 12+ messages in thread
From: Philippe Mathieu-Daudé @ 2024-02-26 12:21 UTC (permalink / raw)
  To: Steve Sistare, qemu-devel
  Cc: Markus Armbruster, Michael Roth, Peter Xu, Fabiano Rosas,
	Marc-André Lureau

On 22/2/24 22:47, Steve Sistare wrote:
> Generalize hmp_split_at_comma() to take any delimiter string, rename
> as str_split(), and move it to util/strList.c.
> 
> No functional change.
> 
> Signed-off-by: Steve Sistare <steven.sistare@oracle.com>
> ---
>   include/monitor/hmp.h  |  1 -
>   include/qemu/strList.h | 24 ++++++++++++++++++++++++
>   monitor/hmp-cmds.c     | 19 -------------------
>   net/net-hmp-cmds.c     |  3 ++-
>   stats/stats-hmp-cmds.c |  3 ++-
>   util/meson.build       |  1 +
>   util/strList.c         | 24 ++++++++++++++++++++++++
>   7 files changed, 53 insertions(+), 22 deletions(-)

>   create mode 100644 include/qemu/strList.h
>   create mode 100644 util/strList.c

Markus, is that OK to include these two in your QAPI
section in MAINTAINERS? Squashing:

-- >8 --
diff --git a/MAINTAINERS b/MAINTAINERS
index 992799171f..7970d34cdd 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -3196,6 +3196,8 @@ X: qapi/*.json
  F: include/qapi/
  X: include/qapi/qmp/
  F: include/qapi/qmp/dispatch.h
+F: include/qemu/strList.h
+F: util/strList.c
  F: tests/qapi-schema/
  F: tests/unit/test-*-visitor.c
  F: tests/unit/test-qapi-*.c
---


^ permalink raw reply related	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2024-02-26 12:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2024-02-22 21:47 [PATCH V5 0/5] string list functions Steve Sistare
2024-02-22 21:47 ` [PATCH V5 1/5] util: str_split Steve Sistare
2024-02-23  5:51   ` Philippe Mathieu-Daudé
2024-02-23  6:01   ` Philippe Mathieu-Daudé
2024-02-23 14:01     ` Steven Sistare
2024-02-23 17:41       ` Philippe Mathieu-Daudé
2024-02-23 18:04         ` Steven Sistare
2024-02-26 12:21   ` Philippe Mathieu-Daudé
2024-02-22 21:47 ` [PATCH V5 2/5] qapi: QAPI_LIST_LENGTH Steve Sistare
2024-02-22 21:47 ` [PATCH V5 3/5] util: strv_from_strList Steve Sistare
2024-02-22 21:47 ` [PATCH V5 4/5] util: strList unit tests Steve Sistare
2024-02-22 21:47 ` [PATCH V5 5/5] migration: simplify exec migration functions Steve Sistare

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).