[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

[libvirt] [PATCH] error: preserve errno when saving last error



It is common to see the sequence:

virErrorPtr save_err = virSaveLastError();
// do cleanup
virSetError(save_err);
virFreeError(save_err);

on cleanup paths.  But for functions where it is desirable to
return the errno that caused failure, this sequence can clobber
that errno.  virFreeError was already safe; this makes the other
two functions in the sequence safe as well, assuming all goes
well (on OOM, errno will be clobbered, but then again, save_err
won't reflect the real error that happened, so you are no longer
preserving the real situation - that's life with OOM).

* src/util/virterror.c (virSaveLastError, virSetError): Preserve
errno.
---

Fixes an errno-clobbering situation pointed out here:
https://www.redhat.com/archives/libvir-list/2011-July/msg01382.html

 src/util/virterror.c |   17 +++++++++++++----
 1 files changed, 13 insertions(+), 4 deletions(-)

diff --git a/src/util/virterror.c b/src/util/virterror.c
index 0c7698a..07f8b45 100644
--- a/src/util/virterror.c
+++ b/src/util/virterror.c
@@ -295,18 +295,24 @@ virGetLastError(void)
  * Can be used to re-set an old error, which may have been squashed by
  * other functions (like cleanup routines).
  *
- * Returns 0 on success, 1 on failure
+ * Returns 0 on success, -1 on failure.  Leaves errno unchanged.
  */
 int
 virSetError(virErrorPtr newerr)
 {
     virErrorPtr err;
+    int saved_errno = errno;
+    int ret = -1;
+
     err = virLastErrorObject();
     if (!err)
-        return -1;
+        goto cleanup;

     virResetError(err);
-    return virCopyError(newerr, err);
+    ret = virCopyError(newerr, err);
+cleanup:
+    errno = saved_errno;
+    return ret;
 }

 /**
@@ -339,7 +345,8 @@ virCopyLastError(virErrorPtr to)
 /**
  * virSaveLastError:
  *
- * Save the last error into a new error object.
+ * Save the last error into a new error object.  On success, errno is
+ * unchanged; on failure, errno is ENOMEM.
  *
  * Returns a pointer to the copied error or NULL if allocation failed.
  * It is the caller's responsibility to free the error with
@@ -349,11 +356,13 @@ virErrorPtr
 virSaveLastError(void)
 {
     virErrorPtr to;
+    int saved_errno = errno;

     if (VIR_ALLOC(to) < 0)
         return NULL;

     virCopyLastError(to);
+    errno = saved_errno;
     return to;
 }

-- 
1.7.4.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]