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

Re: [Xen-devel] [Patch v2] xl: Fix CHK_ERRNO



On 12/09/2013 03:12 PM, Andrew Cooper wrote:
The macro CHK_ERRNO() was being used to check two different error schemes, and
succeeded at neither.

Split the macro into two; CHK_ERRNO() for calls which return -1 and set errno
on error, and CHK_LIBXLERR() for calls which return a positive errno.

In both cases, ensure that strerror() now gets called with the error integer.

Coverity ID: 1055570 1090374 1130516

Signed-off-by: Andrew Cooper <andrew.cooper3@xxxxxxxxxx>
CC: Ian Campbell <Ian.Campbell@xxxxxxxxxx>
CC: Ian Jackson <Ian.Jackson@xxxxxxxxxxxxx>
CC: George Dunlap <george.dunlap@xxxxxxxxxxxxx>

Clearly a bug fix, if a minor one. It does touch the domain creation as well as save/restore path, but is fairly simple and should also be very well tested between now and the release.

Release-acked-by: George Dunlap <george.dunlap@xxxxxxxxxxxxx>


---

Changes in v2:
  * s/CHK_POSERRNO/CHK_LIBXLERR/
---
  tools/libxl/xl_cmdimpl.c |   39 ++++++++++++++++++++++++++-------------
  1 file changed, 26 insertions(+), 13 deletions(-)

diff --git a/tools/libxl/xl_cmdimpl.c b/tools/libxl/xl_cmdimpl.c
index 4977a53..45cb658 100644
--- a/tools/libxl/xl_cmdimpl.c
+++ b/tools/libxl/xl_cmdimpl.c
@@ -41,15 +41,25 @@
  #include "libxlutil.h"
  #include "xl.h"
-#define CHK_ERRNO( call ) ({ \
+/* For calls which return positive errno on failure */
+#define CHK_LIBXLERR( call ) ({                                         \
          int chk_errno = (call);                                         \
-        if (chk_errno < 0) {                                                \
+        if (chk_errno > 0) {                                            \
              fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
                      __FILE__,__LINE__, strerror(chk_errno), #call);     \
              exit(-ERROR_FAIL);                                          \
          }                                                               \
      })
+/* For calls which return -1 and set errno on failure */
+#define CHK_ERRNO( call ) ({                                            \
+        if ((call) == -1) {                                             \
+            fprintf(stderr,"xl: fatal error: %s:%d: %s: %s\n",          \
+                    __FILE__,__LINE__, strerror(errno), #call);         \
+            exit(-ERROR_FAIL);                                          \
+        }                                                               \
+    })
+
  #define MUST( call ) ({                                                 \
          int must_rc = (call);                                           \
          if (must_rc < 0) {                                                  \
@@ -1982,8 +1992,9 @@ static uint32_t create_domain(struct domain_create 
*dom_info)
              if (rc) return rc;
          }
- CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, &hdr,
-                   sizeof(hdr), restore_source, "header") );
+        CHK_LIBXLERR( libxl_read_exactly(
+                          ctx, restore_fd, &hdr, sizeof(hdr),
+                          restore_source, "header") );
          if (memcmp(hdr.magic, savefileheader_magic, sizeof(hdr.magic))) {
              fprintf(stderr, "File has wrong magic number -"
                      " corrupt or for a different tool?\n");
@@ -2008,8 +2019,10 @@ static uint32_t create_domain(struct domain_create 
*dom_info)
          }
          if (hdr.optional_data_len) {
              optdata_begin = xmalloc(hdr.optional_data_len);
-            CHK_ERRNO( libxl_read_exactly(ctx, restore_fd, optdata_begin,
-                   hdr.optional_data_len, restore_source, "optdata") );
+            CHK_LIBXLERR( libxl_read_exactly(
+                              ctx, restore_fd, optdata_begin,
+                              hdr.optional_data_len, restore_source,
+                              "optdata") );
          }
#define OPTDATA_LEFT (hdr.optional_data_len - (optdata_here - optdata_begin))
@@ -3337,9 +3350,9 @@ static void save_domain_core_writeconfig(int fd, const 
char *source,
/* that's the optional data */ - CHK_ERRNO( libxl_write_exactly(ctx, fd,
+    CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
          &hdr, sizeof(hdr), source, "header") );
-    CHK_ERRNO( libxl_write_exactly(ctx, fd,
+    CHK_LIBXLERR( libxl_write_exactly(ctx, fd,
          optdata_begin, hdr.optional_data_len, source, "header") );
fprintf(stderr, "Saving to %s new xl format (info"
@@ -3695,11 +3708,11 @@ static void migrate_receive(int debug, int daemonize, 
int monitor,
fprintf(stderr, "migration target: Ready to receive domain.\n"); - CHK_ERRNO( libxl_write_exactly(ctx, send_fd,
-                                   migrate_receiver_banner,
-                                   sizeof(migrate_receiver_banner)-1,
-                                   "migration ack stream",
-                                   "banner") );
+    CHK_LIBXLERR( libxl_write_exactly(ctx, send_fd,
+                                      migrate_receiver_banner,
+                                      sizeof(migrate_receiver_banner)-1,
+                                      "migration ack stream",
+                                      "banner") );
memset(&dom_info, 0, sizeof(dom_info));
      dom_info.debug = debug;


_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.