Parses return values and returned errors properly.

4 files changed, 151 insertions, 47 deletions

diff --git a/daemon/proto.c b/daemon/proto.c
index 131c66be..62d8da5a 100644
--- a/daemon/proto.c
+++ b/daemon/proto.c
@@ -25,6 +25,7 @@
 #include <unistd.h>
 #include <errno.h>
 #include <ctype.h>
+#include <sys/param.h>		/* defines MIN */
 #include <rpc/types.h>
 #include <rpc/xdr.h>
 
@@ -78,8 +79,6 @@ main_loop (int _sock)
 #if DEBUG
     int i, j;
 
-#define MIN(a,b) ((a)<(b)?(a):(b))
-
     for (i = 0; i < len; i += 16) {
       printf ("%04x: ", i);
       for (j = i; j < MIN (i+16, len); ++j)
diff --git a/src/generator.ml b/src/generator.ml
index f3e53cd9..a54ac3fd 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -243,12 +243,12 @@ and generate_client_actions () =
 
       (* Generate the return value struct. *)
       pr "struct %s_rv {\n" shortname;
-      pr "  int err_code;      /* 0 OK or -1 error */\n";
-      pr "  int serial;        /* serial number of reply */\n";
-      pr "  char err_str[GUESTFS_ERROR_LEN]; /* error from daemon */\n";
+      pr "  int cb_done;  /* flag to indicate callback was called */\n";
+      pr "  struct guestfs_message_header hdr;\n";
+      pr "  struct guestfs_message_error err;\n";
       (match style with
        | (Err, _) -> ()
-    (* | _ -> pr "  struct %s_ret ret;\n" name; REMEMBER TO MEMSET *)
+    (* | _ -> pr "  struct %s_ret ret;\n" name; *)
       );
       pr "};\n\n";
 
@@ -257,8 +257,25 @@ and generate_client_actions () =
       pr "{\n";
       pr "  struct %s_rv *rv = (struct %s_rv *) data;\n" shortname shortname;
       pr "\n";
-      pr "  /* XXX */ rv->err_code = 0;\n";
-      pr "  /* XXX rv->serial = ?; */\n";
+      pr "  if (!xdr_guestfs_message_header (xdr, &rv->hdr)) {\n";
+      pr "    error (g, \"%s: failed to parse reply header\");\n" name;
+      pr "    return;\n";
+      pr "  }\n";
+      pr "  if (rv->hdr.status == GUESTFS_STATUS_ERROR) {\n";
+      pr "    if (!xdr_guestfs_message_error (xdr, &rv->err)) {\n";
+      pr "      error (g, \"%s: failed to parse reply error\");\n" name;
+      pr "      return;\n";
+      pr "    }\n";
+      pr "    goto done;\n";
+      pr "  }\n";
+
+      (match style with
+       | (Err, _) -> ()
+    (* |  _ -> pr "  if (!xdr_%s_ret (&xdr, &rv->ret)) ..." *)
+      );
+
+      pr " done:\n";
+      pr "  rv->cb_done = 1;\n";
       pr "  main_loop.main_loop_quit (g);\n";
       pr "}\n\n";
 
@@ -286,6 +303,9 @@ and generate_client_actions () =
       pr "      g->state);\n";
       pr "    return %s;\n" error_code;
       pr "  }\n";
+      pr "\n";
+      pr "  memset (&rv, 0, sizeof rv);\n";
+      pr "\n";
 
       (match style with
        | (_, P0) ->
@@ -306,23 +326,28 @@ and generate_client_actions () =
       pr "    return %s;\n" error_code;
       pr "\n";
 
-      pr "  rv.err_code = 42;\n";
+      pr "  rv.cb_done = 0;\n";
       pr "  g->reply_cb_internal = %s_cb;\n" shortname;
       pr "  g->reply_cb_internal_data = &rv;\n";
       pr "  main_loop.main_loop_run (g);\n";
       pr "  g->reply_cb_internal = NULL;\n";
       pr "  g->reply_cb_internal_data = NULL;\n";
-      pr "  if (rv.err_code == 42) { /* callback wasn't called */\n";
+      pr "  if (!rv.cb_done) {\n";
       pr "    error (g, \"%s failed, see earlier error messages\");\n" name;
       pr "    return %s;\n" error_code;
       pr "  }\n";
-      pr "  else if (rv.err_code == -1) { /* error from remote end */\n";
-      pr "    error (g, \"%%s\", rv.err_str);\n";
+      pr "\n";
+
+      pr "  if (check_reply_header (g, &rv.hdr, GUESTFS_PROC_%s, serial) == -1)\n"
+	(String.uppercase shortname);
       pr "    return %s;\n" error_code;
-      pr "  }\n";
       pr "\n";
 
-      pr "  /* XXX check serial number agrees */\n\n";
+      pr "  if (rv.hdr.status == GUESTFS_STATUS_ERROR) {\n";
+      pr "    error (g, \"%%s\", rv.err.error);\n";
+      pr "    return %s;\n" error_code;
+      pr "  }\n";
+      pr "\n";
 
       (match style with
        | (Err, _) -> pr "  return 0;\n"
diff --git a/src/guestfs-actions.c b/src/guestfs-actions.c
index f6130d36..fd7a0b92 100644
--- a/src/guestfs-actions.c
+++ b/src/guestfs-actions.c
@@ -20,17 +20,28 @@
  */
 
 struct mount_rv {
-  int err_code;      /* 0 OK or -1 error */
-  int serial;        /* serial number of reply */
-  char err_str[GUESTFS_ERROR_LEN]; /* error from daemon */
+  int cb_done;  /* flag to indicate callback was called */
+  struct guestfs_message_header hdr;
+  struct guestfs_message_error err;
 };
 
 static void mount_cb (guestfs_h *g, void *data, XDR *xdr)
 {
   struct mount_rv *rv = (struct mount_rv *) data;
 
-  /* XXX */ rv->err_code = 0;
-  /* XXX rv->serial = ?; */
+  if (!xdr_guestfs_message_header (xdr, &rv->hdr)) {
+    error (g, "guestfs_mount: failed to parse reply header");
+    return;
+  }
+  if (rv->hdr.status == GUESTFS_STATUS_ERROR) {
+    if (!xdr_guestfs_message_error (xdr, &rv->err)) {
+      error (g, "guestfs_mount: failed to parse reply error");
+      return;
+    }
+    goto done;
+  }
+ done:
+  rv->cb_done = 1;
   main_loop.main_loop_quit (g);
 }
 
@@ -48,6 +59,9 @@ int guestfs_mount (guestfs_h *g,
     return -1;
   }
 
+  memset (&rv, 0, sizeof rv);
+
+
   args.device = (char *) device;
   args.mountpoint = (char *) mountpoint;
   serial = dispatch (g, GUESTFS_PROC_MOUNT,
@@ -55,38 +69,51 @@ int guestfs_mount (guestfs_h *g,
   if (serial == -1)
     return -1;
 
-  rv.err_code = 42;
+  rv.cb_done = 0;
   g->reply_cb_internal = mount_cb;
   g->reply_cb_internal_data = &rv;
   main_loop.main_loop_run (g);
   g->reply_cb_internal = NULL;
   g->reply_cb_internal_data = NULL;
-  if (rv.err_code == 42) { /* callback wasn't called */
+  if (!rv.cb_done) {
     error (g, "guestfs_mount failed, see earlier error messages");
     return -1;
   }
-  else if (rv.err_code == -1) { /* error from remote end */
-    error (g, "%s", rv.err_str);
+
+  if (check_reply_header (g, &rv.hdr, GUESTFS_PROC_MOUNT, serial) == -1)
     return -1;
-  }
 
-  /* XXX check serial number agrees */
+  if (rv.hdr.status == GUESTFS_STATUS_ERROR) {
+    error (g, "%s", rv.err.error);
+    return -1;
+  }
 
   return 0;
 }
 
 struct sync_rv {
-  int err_code;      /* 0 OK or -1 error */
-  int serial;        /* serial number of reply */
-  char err_str[GUESTFS_ERROR_LEN]; /* error from daemon */
+  int cb_done;  /* flag to indicate callback was called */
+  struct guestfs_message_header hdr;
+  struct guestfs_message_error err;
 };
 
 static void sync_cb (guestfs_h *g, void *data, XDR *xdr)
 {
   struct sync_rv *rv = (struct sync_rv *) data;
 
-  /* XXX */ rv->err_code = 0;
-  /* XXX rv->serial = ?; */
+  if (!xdr_guestfs_message_header (xdr, &rv->hdr)) {
+    error (g, "guestfs_sync: failed to parse reply header");
+    return;
+  }
+  if (rv->hdr.status == GUESTFS_STATUS_ERROR) {
+    if (!xdr_guestfs_message_error (xdr, &rv->err)) {
+      error (g, "guestfs_sync: failed to parse reply error");
+      return;
+    }
+    goto done;
+  }
+ done:
+  rv->cb_done = 1;
   main_loop.main_loop_quit (g);
 }
 
@@ -100,42 +127,58 @@ int guestfs_sync (guestfs_h *g)
       g->state);
     return -1;
   }
+
+  memset (&rv, 0, sizeof rv);
+
   serial = dispatch (g, GUESTFS_PROC_SYNC, NULL, NULL);
   if (serial == -1)
     return -1;
 
-  rv.err_code = 42;
+  rv.cb_done = 0;
   g->reply_cb_internal = sync_cb;
   g->reply_cb_internal_data = &rv;
   main_loop.main_loop_run (g);
   g->reply_cb_internal = NULL;
   g->reply_cb_internal_data = NULL;
-  if (rv.err_code == 42) { /* callback wasn't called */
+  if (!rv.cb_done) {
     error (g, "guestfs_sync failed, see earlier error messages");
     return -1;
   }
-  else if (rv.err_code == -1) { /* error from remote end */
-    error (g, "%s", rv.err_str);
+
+  if (check_reply_header (g, &rv.hdr, GUESTFS_PROC_SYNC, serial) == -1)
     return -1;
-  }
 
-  /* XXX check serial number agrees */
+  if (rv.hdr.status == GUESTFS_STATUS_ERROR) {
+    error (g, "%s", rv.err.error);
+    return -1;
+  }
 
   return 0;
 }
 
 struct touch_rv {
-  int err_code;      /* 0 OK or -1 error */
-  int serial;        /* serial number of reply */
-  char err_str[GUESTFS_ERROR_LEN]; /* error from daemon */
+  int cb_done;  /* flag to indicate callback was called */
+  struct guestfs_message_header hdr;
+  struct guestfs_message_error err;
 };
 
 static void touch_cb (guestfs_h *g, void *data, XDR *xdr)
 {
   struct touch_rv *rv = (struct touch_rv *) data;
 
-  /* XXX */ rv->err_code = 0;
-  /* XXX rv->serial = ?; */
+  if (!xdr_guestfs_message_header (xdr, &rv->hdr)) {
+    error (g, "guestfs_touch: failed to parse reply header");
+    return;
+  }
+  if (rv->hdr.status == GUESTFS_STATUS_ERROR) {
+    if (!xdr_guestfs_message_error (xdr, &rv->err)) {
+      error (g, "guestfs_touch: failed to parse reply error");
+      return;
+    }
+    goto done;
+  }
+ done:
+  rv->cb_done = 1;
   main_loop.main_loop_quit (g);
 }
 
@@ -152,28 +195,33 @@ int guestfs_touch (guestfs_h *g,
     return -1;
   }
 
+  memset (&rv, 0, sizeof rv);
+
+
   args.path = (char *) path;
   serial = dispatch (g, GUESTFS_PROC_TOUCH,
                      (xdrproc_t) xdr_guestfs_touch_args, (char *) &args);
   if (serial == -1)
     return -1;
 
-  rv.err_code = 42;
+  rv.cb_done = 0;
   g->reply_cb_internal = touch_cb;
   g->reply_cb_internal_data = &rv;
   main_loop.main_loop_run (g);
   g->reply_cb_internal = NULL;
   g->reply_cb_internal_data = NULL;
-  if (rv.err_code == 42) { /* callback wasn't called */
+  if (!rv.cb_done) {
     error (g, "guestfs_touch failed, see earlier error messages");
     return -1;
   }
-  else if (rv.err_code == -1) { /* error from remote end */
-    error (g, "%s", rv.err_str);
+
+  if (check_reply_header (g, &rv.hdr, GUESTFS_PROC_TOUCH, serial) == -1)
     return -1;
-  }
 
-  /* XXX check serial number agrees */
+  if (rv.hdr.status == GUESTFS_STATUS_ERROR) {
+    error (g, "%s", rv.err.error);
+    return -1;
+  }
 
   return 0;
 }
diff --git a/src/guestfs.c b/src/guestfs.c
index d3594d3b..47af20ad 100644
--- a/src/guestfs.c
+++ b/src/guestfs.c
@@ -1053,6 +1053,38 @@ dispatch (guestfs_h *g, int proc_nr, xdrproc_t xdrp, char *args)
   return -1;
 }
 
+/* Check the return message from a call for validity. */
+static int
+check_reply_header (guestfs_h *g,
+		    const struct guestfs_message_header *hdr,
+		    int proc_nr, int serial)
+{
+  if (hdr->prog != GUESTFS_PROGRAM) {
+    error (g, "wrong program (%d/%d)", hdr->prog, GUESTFS_PROGRAM);
+    return -1;
+  }
+  if (hdr->vers != GUESTFS_PROTOCOL_VERSION) {
+    error (g, "wrong protocol version (%d/%d)",
+	   hdr->vers, GUESTFS_PROTOCOL_VERSION);
+    return -1;
+  }
+  if (hdr->direction != GUESTFS_DIRECTION_REPLY) {
+    error (g, "unexpected message direction (%d/%d)",
+	   hdr->direction, GUESTFS_DIRECTION_REPLY);
+    return -1;
+  }
+  if (hdr->proc != proc_nr) {
+    error (g, "unexpected procedure number (%d/%d)", hdr->proc, proc_nr);
+    return -1;
+  }
+  if (hdr->serial != serial) {
+    error (g, "unexpected serial (%d/%d)", hdr->serial, serial);
+    return -1;
+  }
+
+  return 0;
+}
+
 /* The high-level actions are autogenerated by generator.ml.  Include
  * them here.
  */