Re: [libvirt] PATCH: 9/25: Make global error object thread local

[Jim Meyering <jim meyering net>]

"Daniel P. Berrange" <berrange redhat com> wrote:
> The virGetLastError() and virConnGetLastError() methods are not

Dan,

I suppose you're using mercurial to produce these diffs.
If so, you may want to file a bug, since the diffs
are misleadingly suboptimal.  Here's one example:

-
-    DEBUG("conn=%p", conn);
+    DEBUG("conn=%p", conn);
+
+    virResetLastError();

That's not so bad: since they're short and right next
to each other, it's obvious they're identical.
However, I saw the following one first, and searched visually
for a while before using the editor to confirm that the added/removed
virLibConnError (conn, VIR_ERR_NO_SUPPORT...
lines are identical:

@@ -1217,15 +1235,25 @@ virConnectGetHostname (virConnectPtr con
 {
     DEBUG("conn=%p", conn);

-    if (!VIR_IS_CONNECT(conn)) {
-        virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
-        return NULL;
-    }
-
-    if (conn->driver->getHostname)
-        return conn->driver->getHostname (conn);
-
-    virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+    virResetLastError();
+
+    if (!VIR_IS_CONNECT(conn)) {
+        virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
+        return NULL;
+    }
+
+    if (conn->driver->getHostname) {
+        char *ret = conn->driver->getHostname (conn);
+        if (!ret)
+            goto error;
+        return ret;
+    }
+
+    virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+    /* Copy to connection error object for back compatability */
+    virSetConnError(conn);
     return NULL;
 }

----------------------------------
It's hard enough to read some of these changes without such duplication.
For reference, here are the git-generated diffs:

@@ -1217,15 +1235,25 @@ virConnectGetHostname (virConnectPtr conn)
 {
     DEBUG("conn=%p", conn);

+    virResetLastError();
+
     if (!VIR_IS_CONNECT(conn)) {
         virLibConnError(NULL, VIR_ERR_INVALID_CONN, __FUNCTION__);
         return NULL;
     }

-    if (conn->driver->getHostname)
-        return conn->driver->getHostname (conn);
+    if (conn->driver->getHostname) {
+        char *ret = conn->driver->getHostname (conn);
+        if (!ret)
+            goto error;
+        return ret;
+    }

     virLibConnError (conn, VIR_ERR_NO_SUPPORT, __FUNCTION__);
+
+error:
+    /* Copy to connection error object for back compatability */
+    virSetConnError(conn);
     return NULL;
 }