[snmp PATCH 07/20] libvirtSnmp: Modernize insertGuest

[Michal Privoznik <mprivozn@xxxxxxxxxx>]

Firstly, this style of comparison makes my eyes bleed:

  if (-1 == some_int)

Secondly, the retval is initialized to zero and then in every
error path it is set to -1. Le sigh.
Thirdly, virDomainGetName() can't fail at the point we are
calling it (it can fail iff passed virDomainPtr is invalid, but
that can't be the case).

Signed-off-by: Michal Privoznik <mprivozn@xxxxxxxxxx>
---
 src/libvirtSnmp.c | 68 +++++++++++++++++++++--------------------------
 1 file changed, 31 insertions(+), 37 deletions(-)

diff --git a/src/libvirtSnmp.c b/src/libvirtSnmp.c
index 589fd03..1e7e345 100644
--- a/src/libvirtSnmp.c
+++ b/src/libvirtSnmp.c
@@ -87,54 +87,47 @@ showError(virConnectPtr conn)
 static int
 insertGuest(netsnmp_container *container, virDomainPtr domain)
 {
-    int ret = 0;
     virDomainInfo info;
     libvirtGuestTable_rowreq_ctx *row_ctx = NULL;
-    const char *name = NULL;
-    unsigned char uuid[16];
+    const char *name;
+    unsigned char uuid[VIR_UUID_BUFLEN];
 
-    if (-1 == virDomainGetInfo(domain, &info)) {
+    if (virDomainGetInfo(domain, &info) < 0) {
         printf("Failed to get domain info\n");
         showError(conn);
-        ret = -1;
-        goto out;
+        goto error;
     }
 
     /* create new row in the container */
-    row_ctx = libvirtGuestTable_allocate_rowreq_ctx(NULL);
-    if (!row_ctx) {
+    if (!(row_ctx = libvirtGuestTable_allocate_rowreq_ctx(NULL))) {
         snmp_log(LOG_ERR, "Error creating row");
-        ret = -1;
-        goto out;
+        goto error;
     }
 
     /* set the index of the row */
-    ret = virDomainGetUUID(domain, uuid);
-    if (ret) {
-        snmp_log(LOG_ERR, "Cannot get UUID");
-        libvirtGuestTable_release_rowreq_ctx(row_ctx);
-        ret = -1;
-        goto out;
+    if (virDomainGetUUID(domain, uuid) < 0) {
+        printf("Failed to get domain UUID\n");
+        showError(conn);
+        goto error;
     }
-    if (MFD_SUCCESS != libvirtGuestTable_indexes_set(row_ctx, (char*) uuid,
-                                                     sizeof(uuid))) {
+
+    if (libvirtGuestTable_indexes_set(row_ctx,
+                                      (char*) uuid,
+                                      sizeof(uuid)) != MFD_SUCCESS) {
         snmp_log(LOG_ERR, "Error setting row index");
-        libvirtGuestTable_release_rowreq_ctx(row_ctx);
-        ret = -1;
-        goto out;
+        goto error;
     }
 
     /* set the data */
-    name = virDomainGetName(domain);
-    if (name)
-        row_ctx->data.libvirtGuestName = strdup(name);
-    else
-        row_ctx->data.libvirtGuestName = strdup("");
-    if (!row_ctx->data.libvirtGuestName) {
+    if (!(name = virDomainGetName(domain))) {
+        printf("Failed to get domain name\n");
+        showError(conn);
+        goto error;
+    }
+
+    if (!(row_ctx->data.libvirtGuestName = strdup(name))) {
         snmp_log(LOG_ERR, "Not enough memory for domain name '%s'", name);
-        libvirtGuestTable_release_rowreq_ctx(row_ctx);
-        ret = -1;
-        goto out;
+        goto error;
     }
 
     row_ctx->data.libvirtGuestState = info.state;
@@ -147,16 +140,17 @@ insertGuest(netsnmp_container *container, virDomainPtr domain)
 
     row_ctx->data.libvirtGuestRowStatus = ROWSTATUS_ACTIVE;
 
-    ret = CONTAINER_INSERT(container, row_ctx);
-    if (ret) {
+    if (CONTAINER_INSERT(container, row_ctx) < 0) {
         snmp_log(LOG_ERR, "Cannot insert domain '%s' to container", name);
-        libvirtGuestTable_release_rowreq_ctx(row_ctx);
-        ret = -1;
-        goto out;
+        goto error;
     }
 
- out:
-    return ret;
+    return 0;
+
+ error:
+    if (row_ctx)
+        libvirtGuestTable_release_rowreq_ctx(row_ctx);
+    return -1;
 }
 
 /*
-- 
2.18.1

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