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

[Libguestfs] [PATCH 1/9] daemon error handling: Formalize access to errno.



-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming blog: http://rwmj.wordpress.com
Fedora now supports 80 OCaml packages (the OPEN alternative to F#)
http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
>From cc70a8c8b4c0cae61b0c1268bab56d6817fd23a9 Mon Sep 17 00:00:00 2001
From: Richard Jones <rjones redhat com>
Date: Fri, 27 Nov 2009 13:43:05 +0000
Subject: [PATCH 1/9] daemon error handling: Formalize access to errno.

Windows itself doesn't have an errno variable, although Gnulib
provides a dummy variable (but Windows won't use it, only Gnulib).

In a lot of places in the daemon we access errno directly, eg.
saving and restoring it.  These places aren't safe on Windows.
Instead we provide a simple API for this.

Example, replace:

  int saved_errno = errno;
  do_some_cleanup ();
  errno = saved_errno;
  reply_with_perror ("failed");

with:

  errtype err = get_error ();
  do_some_cleanup ();
  reply_with_perror_with_err (err, "failed");

There's also a method to clear errors (clear_error()).  But we don't
provide a way to set errno to arbitrary values, because the code
doesn't need that and it's hard to do it in Windows.

In some cases you do really need access to errno (not "error"),
either because you know the code won't be used on Win32, or
because Gnulib replacement function sets errno and you want to
check it, so provide also a get_errno function.

Also strerror doesn't exist on Windows, and the Gnulib replacement
only works with network errors.  Replace it with a call to Format-
Message as documented on MSDN.
---
 daemon/daemon.h |   73 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 daemon/proto.c  |   29 +++++++++++++++++++---
 2 files changed, 94 insertions(+), 8 deletions(-)

diff --git a/daemon/daemon.h b/daemon/daemon.h
index e7e77e9..ea1e66f 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -27,6 +27,10 @@
 #include <rpc/types.h>
 #include <rpc/xdr.h>
 
+#ifdef HAVE_WINDOWS_H
+#include <windows.h>
+#endif
+
 #include "../src/guestfs_protocol.h"
 
 /*-- in guestfsd.c --*/
@@ -116,11 +120,72 @@ extern int sync_disks (void);
 /*-- in proto.c --*/
 extern void main_loop (int sock) __attribute__((noreturn));
 
-/* ordinary daemon functions use these to indicate errors */
+/* Don't manipulate errno directly, use these instead.
+ *
+ * For example:
+ *   if (some_syscall (...) == -1) {
+ *     errtype err = get_error ();   // Save the error number.
+ *     do_some_cleanup ();           // This might destroy errno.
+ *     reply_with_perror_with_err (err, ...);  // Report original error.
+ *     return -1;
+ *   }
+ *
+ * The _sock_ variations are only used for socket calls (Win32 has a
+ * different way to return errors from socket calls).  In reality
+ * these variants are never used because the only socket we talk to is
+ * the vmchannel socket, and if that throws an error, we die.
+ */
+#ifndef WIN32
+typedef int errtype;
+static inline void clear_error(void) { errno = 0; }
+static inline int get_error(void) { return errno; }
+static inline int get_sock_error(void) { return errno; }
+#else
+typedef DWORD errtype;
+static inline void clear_error(void)
+{
+  SetLastError (0);
+  WSASetLastError (0);
+  errno = 0;
+}
+static inline DWORD get_error(void) { return GetLastError (); }
+static inline DWORD get_sock_error(void) { return WSAGetLastError (); }
+#endif
+
+/* This is for where you really want _errno_.
+ *
+ * The only case where you should check errno is when U || (GL && GE):
+ *
+ * U = the code only runs on a Unix system
+ * GL = a system call is being replaced by a Gnulib function
+ * GE = the Gnulib function sets errno to some value which you wish to check.
+ *
+ * These cases require careful testing to make sure they work properly on
+ * Windows.
+ */
+static inline int get_errno(void) { return errno; }
+
+/* Finally, discourage direct use of errno. */
+#ifdef errno
+# undef errno
+#endif
+#define errno dont_use_errno_directly_see_daemon_h
+
+/* Ordinary daemon functions use these to indicate errors.
+ * reply_with_error: Report an error.
+ * reply_with_perror: Report an error followed by errno message.
+ * reply_with_perror_sock: Same as reply_with_perror for socket calls.
+ * reply_with_perror_with_err: Same as reply_with_perror but use an
+ *   error code that you saved previously from get_error().
+ */
 extern void reply_with_error (const char *fs, ...)
   __attribute__((format (printf,1,2)));
-extern void reply_with_perror (const char *fs, ...)
-  __attribute__((format (printf,1,2)));
+#define reply_with_perror(...) \
+  reply_with_perror_with_err (get_error(),__VA_ARGS__)
+#define reply_with_perror_sock(...) \
+  reply_with_perror_with_err (get_sock_error(),__VA_ARGS__)
+extern void reply_with_perror_with_err (errtype err, const char *fs, ...)
+  __attribute__((format (printf,2,3)));
 
 /* daemon functions that receive files (FileIn) should call
  * receive_file for each FileIn parameter.
@@ -130,7 +195,7 @@ extern int receive_file (receive_cb cb, void *opaque);
 
 /* daemon functions that receive files (FileIn) can call this
  * to cancel incoming transfers (eg. if there is a local error),
- * but they MUST then call reply_with_error or reply_with_perror.
+ * but they MUST then call reply_with_*.
  */
 extern void cancel_receive (void);
 
diff --git a/daemon/proto.c b/daemon/proto.c
index 2231037..7c923c8 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -23,12 +23,15 @@
 #include <stdarg.h>
 #include <string.h>
 #include <unistd.h>
-#include <errno.h>
 #include <sys/param.h>		/* defines MIN */
 #include <sys/select.h>
 #include <rpc/types.h>
 #include <rpc/xdr.h>
 
+#ifdef HAVE_WINDOWS_H
+#include <windows.h>
+#endif
+
 #include "c-ctype.h"
 #include "ignore-value.h"
 
@@ -180,18 +183,36 @@ reply_with_error (const char *fs, ...)
 }
 
 void
-reply_with_perror (const char *fs, ...)
+reply_with_perror_with_err (errtype err, const char *fs, ...)
 {
   char buf1[GUESTFS_ERROR_LEN];
   char buf2[GUESTFS_ERROR_LEN];
   va_list args;
-  int err = errno;
+  char *errmsg;
 
   va_start (args, fs);
   vsnprintf (buf1, sizeof buf1, fs, args);
   va_end (args);
 
-  snprintf (buf2, sizeof buf2, "%s: %s", buf1, strerror (err));
+#ifdef WIN32
+  /* http://msdn.microsoft.com/en-us/library/ms680582%28VS.85%29.aspx */
+  FormatMessage (
+        FORMAT_MESSAGE_ALLOCATE_BUFFER | 
+        FORMAT_MESSAGE_FROM_SYSTEM |
+        FORMAT_MESSAGE_IGNORE_INSERTS,
+        NULL,
+        err,
+        MAKELANGID (LANG_NEUTRAL, SUBLANG_DEFAULT),
+        (LPTSTR) &errmsg,
+        0, NULL);
+#else
+  errmsg = strerror (err);
+#endif
+  snprintf (buf2, sizeof buf2, "%s: %s", buf1, errmsg);
+
+#ifdef WIN32
+  LocalFree (errmsg);
+#endif
 
   send_error (buf2);
 }
-- 
1.6.5.2


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