From c59176c109457161861fd883111373410d3b3a66 Mon Sep 17 00:00:00 2001
From: "Daniel P. Berrange" <berrange@redhat.com>
Date: Thu, 17 Mar 2011 15:54:24 +0000
Subject: [PATCH] Fix uninitialized variable & error reporting in LXC veth
 setup

THe veth setup in LXC had a couple of flaws, first brInit did
not report any error when it failed. Second vethCreate() did
not correctly initialize the variable containing the return
code, so could report failure even when it succeeded.

* src/lxc/lxc_driver.c: Report error when brInit fails
* src/lxc/veth.c: Fix uninitialized variable
---
 src/lxc/lxc_driver.c |  8 ++++++--
 src/lxc/veth.c       | 33 ++++++++++++++++++++++-----------
 2 files changed, 28 insertions(+), 13 deletions(-)

diff --git a/src/lxc/lxc_driver.c b/src/lxc/lxc_driver.c
index d5f21b1c34..3159e1b4db 100644
--- a/src/lxc/lxc_driver.c
+++ b/src/lxc/lxc_driver.c
@@ -1024,9 +1024,13 @@ static int lxcSetupInterfaces(virConnectPtr conn,
     int rc = -1, i;
     char *bridge = NULL;
     brControl *brctl = NULL;
+    int ret;
 
-    if (brInit(&brctl) != 0)
+    if ((ret = brInit(&brctl)) != 0) {
+        virReportSystemError(ret, "%s",
+                             _("Unable to initialize bridging"));
         return -1;
+    }
 
     for (i = 0 ; i < def->nnets ; i++) {
         char *parentVeth;
@@ -1095,7 +1099,7 @@ static int lxcSetupInterfaces(virConnectPtr conn,
                 goto error_exit;
         }
 
-        if (0 != (rc = brAddInterface(brctl, bridge, parentVeth))) {
+        if ((ret = brAddInterface(brctl, bridge, parentVeth)) != 0) {
             virReportSystemError(rc,
                                  _("Failed to add %s device to %s"),
                                  parentVeth, bridge);
diff --git a/src/lxc/veth.c b/src/lxc/veth.c
index 0fa76cfc74..26bf4ff88b 100644
--- a/src/lxc/veth.c
+++ b/src/lxc/veth.c
@@ -90,33 +90,40 @@ static int getFreeVethName(char **veth, int startDev)
  */
 int vethCreate(char** veth1, char** veth2)
 {
-    int rc;
+    int rc = -1;
     const char *argv[] = {
         "ip", "link", "add", NULL, "type", "veth", "peer", "name", NULL, NULL
     };
     int vethDev = 0;
     bool veth1_alloc = false;
+    bool veth2_alloc = false;
 
     VIR_DEBUG("veth1: %s veth2: %s", NULLSTR(*veth1), NULLSTR(*veth2));
 
     if (*veth1 == NULL) {
-        vethDev = getFreeVethName(veth1, vethDev);
-        if (vethDev < 0)
-            return vethDev;
+        if ((vethDev = getFreeVethName(veth1, vethDev)) < 0)
+            goto cleanup;
         VIR_DEBUG("Assigned veth1: %s", *veth1);
         veth1_alloc = true;
     }
     argv[3] = *veth1;
 
-    while (*veth2 == NULL || STREQ(*veth1, *veth2)) {
-        VIR_FREE(*veth2);
-        vethDev = getFreeVethName(veth2, vethDev + 1);
-        if (vethDev < 0) {
+    while (*veth2 == NULL) {
+        if ((vethDev = getFreeVethName(veth2, vethDev + 1)) < 0) {
             if (veth1_alloc)
                 VIR_FREE(*veth1);
-            return vethDev;
+            goto cleanup;
         }
+
+        /* Just make sure they didn't accidentally get same name */
+        if (STREQ(*veth1, *veth2)) {
+            vethDev++;
+            VIR_FREE(*veth2);
+            continue;
+        }
+
         VIR_DEBUG("Assigned veth2: %s", *veth2);
+        veth2_alloc = true;
     }
     argv[8] = *veth2;
 
@@ -124,10 +131,14 @@ int vethCreate(char** veth1, char** veth2)
     if (virRun(argv, NULL) < 0) {
         if (veth1_alloc)
             VIR_FREE(*veth1);
-        VIR_FREE(*veth2);
-        rc = -1;
+        if (veth2_alloc)
+            VIR_FREE(*veth2);
+        goto cleanup;
     }
 
+    rc = 0;
+
+cleanup:
     return rc;
 }