Many places in the code call virGetLastError() just to check the
raised error code, or domain. However virGetLastError() can return
NULL, so the code has to check for that first. This patch therefore
introduces virGetLasErrorCode/Domain function which always returns a
valid error code/domain respectively, thus dropping the need to
perform any checks on the error object.

Signed-off-by: Ramy Elkest <ramyelkest@gmail.com>
---
 include/libvirt/virterror.h |  2 ++
 src/libvirt_public.syms     |  6 ++++++
 src/util/virerror.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 50 insertions(+)

diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
index 3e7c7a0..5e58b6a 100644
--- a/include/libvirt/virterror.h
+++ b/include/libvirt/virterror.h
@@ -344,6 +344,8 @@ void                    virResetLastError       (void);
 void                    virResetError           (virErrorPtr err);
 void                    virFreeError            (virErrorPtr err);
 
+int                     virGetLastErrorCode     (void);
+int                     virGetLastErrorDomain   (void);
 const char *            virGetLastErrorMessage  (void);
 
 virErrorPtr             virConnGetLastError     (virConnectPtr conn);
diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
index 95df3a0..85b6b6d 100644
--- a/src/libvirt_public.syms
+++ b/src/libvirt_public.syms
@@ -785,4 +785,10 @@ LIBVIRT_4.1.0 {
         virStoragePoolLookupByTargetPath;
 } LIBVIRT_3.9.0;
 
+LIBVIRT_4.4.0 {
+    global:
+        virGetLastErrorCode;
+        virGetLastErrorDomain;
+} LIBVIRT_4.1.0;
+
 # .... define new API here using predicted next version number ....
diff --git a/src/util/virerror.c b/src/util/virerror.c
index c000b00..842fc49 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -272,6 +272,48 @@ virGetLastError(void)
 
 
 /**
+ * virGetLastErrorCode:
+ *
+ * Get the most recent error code
+ *
+ * The error object is kept in thread local storage, so separate
+ * threads can safely access this concurrently.
+ *
+ * Returns the most recent error code in this thread,
+ * or VIR_ERR_OK if none is set
+ */
+int
+virGetLastErrorCode(void)
+{
+    virErrorPtr err = virLastErrorObject();
+    if (!err)
+        return VIR_ERR_OK;
+    return err->code;
+}
+
+
+/**
+ * virGetLastErrorDomain:
+ *
+ * Get the most recent error domain
+ *
+ * The error object is kept in thread local storage, so separate
+ * threads can safely access this concurrently.
+ *
+ * Returns the most recent error code in this thread,
+ * or VIR_FROM_NONE if none is set
+ */
+int
+virGetLastErrorDomain(void)
+{
+    virErrorPtr err = virLastErrorObject();
+    if (!err)
+        return VIR_FROM_NONE;
+    return err->domain;
+}
+
+
+/**
  * virGetLastErrorMessage:
  *
  * Get the most recent error message
-- 
2.7.4

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

On Sat, May 05, 2018 at 01:04:20PM +0100, ramyelkest wrote:
> Many places in the code call virGetLastError() just to check the
> raised error code, or domain. However virGetLastError() can return
> NULL, so the code has to check for that first. This patch therefore
> introduces virGetLasErrorCode/Domain function which always returns a
> valid error code/domain respectively, thus dropping the need to
> perform any checks on the error object.
>
> Signed-off-by: Ramy Elkest <ramyelkest@gmail.com>
> ---
>  include/libvirt/virterror.h |  2 ++
>  src/libvirt_public.syms     |  6 ++++++
>  src/util/virerror.c         | 42 ++++++++++++++++++++++++++++++++++++++++++
>  3 files changed, 50 insertions(+)
>
> diff --git a/include/libvirt/virterror.h b/include/libvirt/virterror.h
> index 3e7c7a0..5e58b6a 100644
> --- a/include/libvirt/virterror.h
> +++ b/include/libvirt/virterror.h
> @@ -344,6 +344,8 @@ void                    virResetLastError       (void);
>  void                    virResetError           (virErrorPtr err);
>  void                    virFreeError            (virErrorPtr err);
>
> +int                     virGetLastErrorCode     (void);
> +int                     virGetLastErrorDomain   (void);
>  const char *            virGetLastErrorMessage  (void);
>
>  virErrorPtr             virConnGetLastError     (virConnectPtr conn);
> diff --git a/src/libvirt_public.syms b/src/libvirt_public.syms
> index 95df3a0..85b6b6d 100644
> --- a/src/libvirt_public.syms
> +++ b/src/libvirt_public.syms
> @@ -785,4 +785,10 @@ LIBVIRT_4.1.0 {
>          virStoragePoolLookupByTargetPath;
>  } LIBVIRT_3.9.0;
>
> +LIBVIRT_4.4.0 {
> +    global:
> +        virGetLastErrorCode;
> +        virGetLastErrorDomain;
> +} LIBVIRT_4.1.0;
> +
>  # .... define new API here using predicted next version number ....
> diff --git a/src/util/virerror.c b/src/util/virerror.c
> index c000b00..842fc49 100644
> --- a/src/util/virerror.c
> +++ b/src/util/virerror.c
> @@ -272,6 +272,48 @@ virGetLastError(void)
>
>
>  /**
> + * virGetLastErrorCode:
> + *
> + * Get the most recent error code
> + *
> + * The error object is kept in thread local storage, so separate
> + * threads can safely access this concurrently.

A tiny detail I missed during v1, we don't really need to mention ^this bit,
we'd only explicitly document if an API wasn't thread safe, the internal facts
should be transparent to users, it should *just* work, same applies to the hunk
below, I'd suggest squashing in the following so that you don't have to send a
v3:

diff --git a/src/util/virerror.c b/src/util/virerror.c
index 842fc493f1..93632dbdf7 100644
--- a/src/util/virerror.c
+++ b/src/util/virerror.c
@@ -274,13 +274,9 @@ virGetLastError(void)
 /**
  * virGetLastErrorCode:
  *
- * Get the most recent error code
+ * Get the most recent error code (enum virErrorNumber).
  *
- * The error object is kept in thread local storage, so separate
- * threads can safely access this concurrently.
- *
- * Returns the most recent error code in this thread,
- * or VIR_ERR_OK if none is set
+ * Returns the most recent error code, or VIR_ERR_OK if none is set.
  */
 int
 virGetLastErrorCode(void)
@@ -295,13 +291,10 @@ virGetLastErrorCode(void)
 /**
  * virGetLastErrorDomain:
  *
- * Get the most recent error domain
+ * Get the most recent error domain (enum virErrorDomain).
  *
- * The error object is kept in thread local storage, so separate
- * threads can safely access this concurrently.
- *
- * Returns the most recent error code in this thread,
- * or VIR_FROM_NONE if none is set
+ * Returns a numerical value of the most recent error's origin, or VIR_FROM_NONE
+ * if none is set.
  */
 int
 virGetLastErrorDomain(void)

Other than that, you have my
Reviewed-by: Erik Skultety <eskultet@redhat.com>

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