Re: [PATCH 09/11] tools/xc: pass errno to callers of xc_domain_save

[Olaf Hering <olaf@aepfle.de>]

On Thu, Mar 06, Ian Jackson wrote:

> Olaf Hering writes ("[PATCH 09/11] tools/xc: pass errno to callers of xc_domain_save"):
> > Callers of xc_domain_save use errno to print diagnostics if the call
> > fails. But xc_domain_save does not preserve the actual errno in case of
> > a failure.
> > 
> > This change preserves errno in all cases where code jumps to the label
> > "out". In addition a new label "exit" is added to catch also code which
> > used to do just "return 1".
> 
> I can't help wondering if this patch would be a lot smaller if it were
> done by having ERROR and PERROR, and the cleanup part of
> xc_domain_save, preserve errno.

Something like this for the macros?

diff --git a/tools/libxc/xc_private.h b/tools/libxc/xc_private.h
index a610f0c..8b791b6 100644
--- a/tools/libxc/xc_private.h
+++ b/tools/libxc/xc_private.h
@@ -123,9 +123,15 @@ void xc_report_progress_step(xc_interface *xch,
 #define DPRINTF(_f, _a...) xc_report(xch, xch->error_handler, XTL_DETAIL,0, _f , ## _a)
 #define DBGPRINTF(_f, _a...) xc_report(xch, xch->error_handler, XTL_DEBUG,0, _f , ## _a)
 
-#define ERROR(_m, _a...)  xc_report_error(xch,XC_INTERNAL_ERROR,_m , ## _a )
-#define PERROR(_m, _a...) xc_report_error(xch,XC_INTERNAL_ERROR,_m \
-                  " (%d = %s)", ## _a , errno, xc_strerror(xch, errno))
+#define ERROR(_m, _a...)  do { int __errno = errno; \
+        xc_report_error(xch,XC_INTERNAL_ERROR,_m , ## _a ); \
+        errno = __errno; \
+        } while (0)
+#define PERROR(_m, _a...) do { int __errno = errno; \
+        xc_report_error(xch,XC_INTERNAL_ERROR,_m " (%d = %s)", \
+        ## _a , errno, xc_strerror(xch, errno)); \
+        errno = __errno; \
+        } while (0)
 
 /*
  * HYPERCALL ARGUMENT BUFFERS


> > Note: some of the functions used in xc_domain_save do not use errno to
> > indicate a reason. In these cases the errno remains undefined as it used
> > to be without this change.
> 
> Do you intend to fix this too ?

I will have a look.

Olaf