[libvirt] [PATCH] domain_conf.c: avoid a leak and the need for "cleanup:" block

[Jim Meyering]

This avoids a leak and the need for a "cleanup:" block,
along with its three goto statements.

While often I prefer to write functions with a single return point,
this one no longer has the need, now that "addr" is freed immediately
after allocation.

Adding the semicolon in the "case..." stmt may look odd.
It's there because the first stmt is the declaration of "port".
If you'd prefer, an alternative is to put the contents of that
case inside a {...} block.

>From dc879b00ccf6cc451b95ca3712e647605579ee0b Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Wed, 20 Jan 2010 19:27:43 +0100
Subject: [PATCH] domain_conf.c: avoid a leak and the need for "cleanup:" block

* src/conf/domain_conf.c (virDomainChrDefFormat): Plug a leak on
an error path, and at the same time, eliminate the need for a
"cleanup:" block.  Before, the "return -1" after the switch
would leak an "addr" string.  Now, by reversing the port,addr-
getting blocks we can free "addr" immediately and skip the goto.
---
 src/conf/domain_conf.c |   30 +++++++++++++-----------------
 1 files changed, 13 insertions(+), 17 deletions(-)

diff --git a/src/conf/domain_conf.c b/src/conf/domain_conf.c
index 6254dc8..7815805 100644
--- a/src/conf/domain_conf.c
+++ b/src/conf/domain_conf.c
@@ -1,7 +1,7 @@
 /*
  * domain_conf.c: domain XML processing
  *
- * Copyright (C) 2006-2009 Red Hat, Inc.
+ * Copyright (C) 2006-2010 Red Hat, Inc.
  * Copyright (C) 2006-2008 Daniel P. Berrange
  *
  * This library is free software; you can redistribute it and/or
@@ -4831,7 +4831,6 @@ virDomainChrDefFormat(virConnectPtr conn,
     const char *targetName = virDomainChrTargetTypeToString(def->targetType);
     const char *elementName;

-    const char *addr = NULL;
     int ret = 0;

     switch (def->targetType) {
@@ -4847,8 +4846,7 @@ virDomainChrDefFormat(virConnectPtr conn,
     if (!type) {
         virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR,
                              _("unexpected char type %d"), def->type);
-        ret = -1;
-        goto cleanup;
+        return -1;
     }

     /* Compat with legacy  <console tty='/dev/pts/5'/> syntax */
@@ -4931,22 +4929,23 @@ virDomainChrDefFormat(virConnectPtr conn,

     switch (def->targetType) {
     case VIR_DOMAIN_CHR_TARGET_TYPE_GUESTFWD:
-        addr = virSocketFormatAddr(def->target.addr);
-        if (addr == NULL) {
-            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
-                                 _("Unable to format guestfwd address"));
-            ret = -1;
-            goto cleanup;
-        }
+        ; /* dummy stmt, for following declaration */
         int port = virSocketGetPort(def->target.addr);
         if (port < 0) {
             virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
                                  _("Unable to format guestfwd port"));
-            ret = -1;
-            goto cleanup;
+            return -1;
+        }
+        const char *addr = virSocketFormatAddr(def->target.addr);
+        if (addr == NULL) {
+            virDomainReportError(conn, VIR_ERR_INTERNAL_ERROR, "%s",
+                                 _("Unable to format guestfwd address"));
+            return -1;
         }
-        virBufferVSprintf(buf, "      <target type='guestfwd' address='%s' port='%d'/>\n",
+        virBufferVSprintf(buf,
+                    "      <target type='guestfwd' address='%s' port='%d'/>\n",
                           addr, port);
+        VIR_FREE(addr);
         break;

     case VIR_DOMAIN_CHR_TARGET_TYPE_PARALLEL:
@@ -4969,9 +4968,6 @@ virDomainChrDefFormat(virConnectPtr conn,
     virBufferVSprintf(buf, "    </%s>\n",
                       elementName);

-cleanup:
-    VIR_FREE(addr);
-
     return ret;
 }

--
1.6.6.516.gb72f