From 1a02760cc6d79422dc4dd17b943915cd0ac15987 Mon Sep 17 00:00:00 2001
From: Peter Krempa <pkrempa@redhat.com>
Date: Mon, 27 Jan 2025 17:38:51 +0100
Subject: [PATCH] virDomainObjGetMessages: Refactor using GPtrArray
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Simplify the function especially by rewriting it using GPtrArray to
construct the string list, especially for the upcoming case when the
number of added elements will not be known beforehand and when
hypervisor specific data will be added.

Signed-off-by: Peter Krempa <pkrempa@redhat.com>
Reviewed-by: Daniel P. Berrangé <berrange@redhat.com>
---
 src/conf/domain_conf.c   | 36 +++++++-----------------------------
 src/conf/domain_conf.h   |  4 ++--
 src/libxl/libxl_driver.c |  9 ++++++++-
 src/qemu/qemu_driver.c   |  9 ++++++++-
 src/test/test_driver.c   |  9 ++++++++-
 5 files changed, 33 insertions(+), 34 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 45c2cd09f1..00d486e774 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -31685,53 +31685,31 @@ virHostdevIsPCIDevice(const virDomainHostdevDef *hostdev)
 /**
  * virDomainObjGetMessages:
  * @vm: domain object
- * @msgs: pointer to a variable to store messages
+ * @m: GPtrArray to be filled with messages
  * @flags: zero or more virDomainMessageType flags
- *
- * Returns number of messages stored in @msgs, -1 otherwise.
  */
-int
+void
 virDomainObjGetMessages(virDomainObj *vm,
-                        char ***msgs,
+                        GPtrArray *m,
                         unsigned int flags)
 {
     size_t i = 0;
-    size_t n = 0;
-    int nmsgs = 0;
-    int rv = -1;
-
-    *msgs = NULL;
 
     if (!flags || (flags & VIR_DOMAIN_MESSAGE_TAINTING)) {
-        nmsgs += __builtin_popcount(vm->taint);
-        *msgs = g_renew(char *, *msgs, nmsgs+1);
-
         for (i = 0; i < VIR_DOMAIN_TAINT_LAST; i++) {
             if (vm->taint & (1 << i)) {
-                (*msgs)[n++] = g_strdup_printf(
-                    _("tainted: %1$s"),
-                    _(virDomainTaintMessageTypeToString(i)));
+                g_ptr_array_add(m, g_strdup_printf(_("tainted: %1$s"),
+                                                   _(virDomainTaintMessageTypeToString(i))));
             }
         }
     }
 
     if (!flags || (flags & VIR_DOMAIN_MESSAGE_DEPRECATION)) {
-        nmsgs += vm->ndeprecations;
-        *msgs = g_renew(char *, *msgs, nmsgs+1);
-
         for (i = 0; i < vm->ndeprecations; i++) {
-            (*msgs)[n++] = g_strdup_printf(
-                _("deprecated configuration: %1$s"),
-                vm->deprecations[i]);
+            g_ptr_array_add(m, g_strdup_printf(_("deprecated configuration: %1$s"),
+                                               vm->deprecations[i]));
         }
     }
-
-    if (*msgs)
-        (*msgs)[nmsgs] = NULL;
-
-    rv = nmsgs;
-
-    return rv;
 }
 
 bool
diff --git a/src/conf/domain_conf.h b/src/conf/domain_conf.h
index ba1a495764..e996d3c0de 100644
--- a/src/conf/domain_conf.h
+++ b/src/conf/domain_conf.h
@@ -4588,9 +4588,9 @@ bool
 virHostdevIsPCIDevice(const virDomainHostdevDef *hostdev)
     ATTRIBUTE_NONNULL(1);
 
-int
+void
 virDomainObjGetMessages(virDomainObj *vm,
-                        char ***msgs,
+                        GPtrArray *m,
                         unsigned int flags);
 
 bool
diff --git a/src/libxl/libxl_driver.c b/src/libxl/libxl_driver.c
index 058fee0706..426c2b4278 100644
--- a/src/libxl/libxl_driver.c
+++ b/src/libxl/libxl_driver.c
@@ -6575,6 +6575,7 @@ libxlDomainGetMessages(virDomainPtr dom,
                       char ***msgs,
                       unsigned int flags)
 {
+    g_autoptr(GPtrArray) m = g_ptr_array_new_with_free_func(g_free);
     virDomainObj *vm = NULL;
     int ret = -1;
 
@@ -6587,7 +6588,13 @@ libxlDomainGetMessages(virDomainPtr dom,
     if (virDomainGetMessagesEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    ret = virDomainObjGetMessages(vm, msgs, flags);
+    virDomainObjGetMessages(vm, m, flags);
+
+    ret = m->len;
+    if (m->len > 0) {
+        g_ptr_array_add(m, NULL);
+        *msgs = (char **) g_ptr_array_steal(m, NULL);
+    }
 
  cleanup:
     virDomainObjEndAPI(&vm);
diff --git a/src/qemu/qemu_driver.c b/src/qemu/qemu_driver.c
index f326937585..72dd471b6e 100644
--- a/src/qemu/qemu_driver.c
+++ b/src/qemu/qemu_driver.c
@@ -19834,6 +19834,7 @@ qemuDomainGetMessages(virDomainPtr dom,
                       char ***msgs,
                       unsigned int flags)
 {
+    g_autoptr(GPtrArray) m = g_ptr_array_new_with_free_func(g_free);
     virDomainObj *vm = NULL;
     int rv = -1;
 
@@ -19846,7 +19847,13 @@ qemuDomainGetMessages(virDomainPtr dom,
     if (virDomainGetMessagesEnsureACL(dom->conn, vm->def) < 0)
         goto cleanup;
 
-    rv = virDomainObjGetMessages(vm, msgs, flags);
+    virDomainObjGetMessages(vm, m, flags);
+
+    rv = m->len;
+    if (m->len > 0) {
+        g_ptr_array_add(m, NULL);
+        *msgs = (char **) g_ptr_array_steal(m, NULL);
+    }
 
  cleanup:
     virDomainObjEndAPI(&vm);
diff --git a/src/test/test_driver.c b/src/test/test_driver.c
index f1cefb5c50..a10ec3bc41 100644
--- a/src/test/test_driver.c
+++ b/src/test/test_driver.c
@@ -9522,6 +9522,7 @@ testDomainGetMessages(virDomainPtr dom,
                       char ***msgs,
                       unsigned int flags)
 {
+    g_autoptr(GPtrArray) m = g_ptr_array_new_with_free_func(g_free);
     virDomainObj *vm = NULL;
     int rv = -1;
 
@@ -9531,7 +9532,13 @@ testDomainGetMessages(virDomainPtr dom,
     if (!(vm = testDomObjFromDomain(dom)))
         return -1;
 
-    rv = virDomainObjGetMessages(vm, msgs, flags);
+    virDomainObjGetMessages(vm, m, flags);
+
+    rv = m->len;
+    if (m->len > 0) {
+        g_ptr_array_add(m, NULL);
+        *msgs = (char **) g_ptr_array_steal(m, NULL);
+    }
 
     virDomainObjEndAPI(&vm);
     return rv;