[libvirt] [RFC PATCH] qemumonitorjsontest: Use VIR_AUTOPTR for test cleanup

Eric Blake posted 1 patch 13 weeks ago
Failed in applying to current master (apply log)
tests/qemumonitortestutils.h |  3 +++
tests/qemumonitorjsontest.c  | 35 +++++++++++++++--------------------
2 files changed, 18 insertions(+), 20 deletions(-)

[libvirt] [RFC PATCH] qemumonitorjsontest: Use VIR_AUTOPTR for test cleanup

Posted by Eric Blake 13 weeks ago
Everywhere else is switching, we might as well use it here, too.

Signed-off-by: Eric Blake <eblake@redhat.com>

---

Partial patch - is it worth me continuing the cleanups on the rest of
the file?
---
 tests/qemumonitortestutils.h |  3 +++
 tests/qemumonitorjsontest.c  | 35 +++++++++++++++--------------------
 2 files changed, 18 insertions(+), 20 deletions(-)

diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
index 8461c80caa..a2d2d30820 100644
--- a/tests/qemumonitortestutils.h
+++ b/tests/qemumonitortestutils.h
@@ -24,6 +24,7 @@
 # include "qemu/qemu_conf.h"
 # include "qemu/qemu_monitor.h"
 # include "qemu/qemu_agent.h"
+# include "virautoclean.h"

 typedef struct _qemuMonitorTest qemuMonitorTest;
 typedef qemuMonitorTest *qemuMonitorTestPtr;
@@ -102,4 +103,6 @@ qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test);
 qemuAgentPtr qemuMonitorTestGetAgent(qemuMonitorTestPtr test);
 virDomainObjPtr qemuMonitorTestGetDomainObj(qemuMonitorTestPtr test);

+VIR_DEFINE_AUTOPTR_FUNC(qemuMonitorTest, qemuMonitorTestFree);
+
 #endif /* LIBVIRT_QEMUMONITORTESTUTILS_H */
diff --git a/tests/qemumonitorjsontest.c b/tests/qemumonitorjsontest.c
index a7f64058d4..10bffce2a3 100644
--- a/tests/qemumonitorjsontest.c
+++ b/tests/qemumonitorjsontest.c
@@ -152,12 +152,11 @@ testQemuMonitorJSONGetStatus(const void *opaque)
 {
     const testGenericData *data = opaque;
     virDomainXMLOptionPtr xmlopt = data->xmlopt;
-    qemuMonitorTestPtr test = qemuMonitorTestNewSchema(xmlopt, data->schema);
-    int ret = -1;
+    VIR_AUTOPTR(qemuMonitorTest) test = NULL;
     bool running = false;
     virDomainPausedReason reason = 0;

-    if (!test)
+    if (!(test = qemuMonitorTestNewSchema(xmlopt, data->schema)))
         return -1;

     if (qemuMonitorTestAddItem(test, "query-status",
@@ -168,7 +167,7 @@ testQemuMonitorJSONGetStatus(const void *opaque)
                                "        \"running\": true "
                                "    } "
                                "}") < 0)
-        goto cleanup;
+        return -1;
     if (qemuMonitorTestAddItem(test, "query-status",
                                "{ "
                                "    \"return\": { "
@@ -176,7 +175,7 @@ testQemuMonitorJSONGetStatus(const void *opaque)
                                "        \"running\": false "
                                "    } "
                                "}") < 0)
-        goto cleanup;
+        return -1;
     if (qemuMonitorTestAddItem(test, "query-status",
                                "{ "
                                "    \"return\": { "
@@ -185,61 +184,57 @@ testQemuMonitorJSONGetStatus(const void *opaque)
                                "        \"running\": false "
                                "    } "
                                "}") < 0)
-        goto cleanup;
+        return -1;

     if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test),
                              &running, &reason) < 0)
-        goto cleanup;
+        return -1;

     if (!running) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Running was not true");
-        goto cleanup;
+        return -1;
     }

     if (reason != VIR_DOMAIN_PAUSED_UNKNOWN) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "Reason was unexpectedly set to %d", reason);
-        goto cleanup;
+        return -1;
     }

     if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test),
                              &running, &reason) < 0)
-        goto cleanup;
+        return -1;

     if (running) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Running was not false");
-        goto cleanup;
+        return -1;
     }

     if (reason != VIR_DOMAIN_PAUSED_UNKNOWN) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "Reason was unexpectedly set to %d", reason);
-        goto cleanup;
+        return -1;
     }

     if (qemuMonitorGetStatus(qemuMonitorTestGetMonitor(test),
                              &running, &reason) < 0)
-        goto cleanup;
+        return -1;

     if (running) {
         virReportError(VIR_ERR_INTERNAL_ERROR, "%s",
                        "Running was not false");
-        goto cleanup;
+        return -1;
     }

     if (reason != VIR_DOMAIN_PAUSED_MIGRATION) {
         virReportError(VIR_ERR_INTERNAL_ERROR,
                        "Reason was unexpectedly set to %d", reason);
-        goto cleanup;
+        return -1;
     }

-    ret = 0;
-
- cleanup:
-    qemuMonitorTestFree(test);
-    return ret;
+    return 0;
 }

 static int
-- 
2.20.1

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list

Re: [libvirt] [RFC PATCH] qemumonitorjsontest: Use VIR_AUTOPTR for test cleanup

Posted by Peter Krempa 13 weeks ago
On Mon, Jun 10, 2019 at 22:03:05 -0500, Eric Blake wrote:
> Everywhere else is switching, we might as well use it here, too.
> 
> Signed-off-by: Eric Blake <eblake@redhat.com>
> 
> ---
> 
> Partial patch - is it worth me continuing the cleanups on the rest of
> the file?

ACK to this one if you split it, but I'm unsure whether it's worth
investing time into converting all of the tests. Definitely the
definition of the AUTOPTR function is worth because new tests can use
the new format.

> ---
>  tests/qemumonitortestutils.h |  3 +++
>  tests/qemumonitorjsontest.c  | 35 +++++++++++++++--------------------
>  2 files changed, 18 insertions(+), 20 deletions(-)
> 
> diff --git a/tests/qemumonitortestutils.h b/tests/qemumonitortestutils.h
> index 8461c80caa..a2d2d30820 100644
> --- a/tests/qemumonitortestutils.h
> +++ b/tests/qemumonitortestutils.h
> @@ -24,6 +24,7 @@
>  # include "qemu/qemu_conf.h"
>  # include "qemu/qemu_monitor.h"
>  # include "qemu/qemu_agent.h"
> +# include "virautoclean.h"
> 
>  typedef struct _qemuMonitorTest qemuMonitorTest;
>  typedef qemuMonitorTest *qemuMonitorTestPtr;
> @@ -102,4 +103,6 @@ qemuMonitorPtr qemuMonitorTestGetMonitor(qemuMonitorTestPtr test);
>  qemuAgentPtr qemuMonitorTestGetAgent(qemuMonitorTestPtr test);
>  virDomainObjPtr qemuMonitorTestGetDomainObj(qemuMonitorTestPtr test);
> 
> +VIR_DEFINE_AUTOPTR_FUNC(qemuMonitorTest, qemuMonitorTestFree);

Possibly add this in a separate patch?

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list