From 316bbc36662c0df6b3d0ad48790e0b551a291df6 Mon Sep 17 00:00:00 2001 From: Richard Jones Date: Fri, 3 Apr 2009 19:51:02 +0100 Subject: [PATCH] Parses return values and returned errors properly. --- daemon/proto.c | 3 +- src/generator.ml | 49 ++++++++++++++++------ src/guestfs-actions.c | 114 +++++++++++++++++++++++++++++++++++--------------- src/guestfs.c | 32 ++++++++++++++ 4 files changed, 151 insertions(+), 47 deletions(-) diff --git a/daemon/proto.c b/daemon/proto.c index 131c66b..62d8da5 100644 --- a/daemon/proto.c +++ b/daemon/proto.c @@ -25,6 +25,7 @@ #include #include #include +#include /* defines MIN */ #include #include @@ -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 f3e53cd..a54ac3f 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 f6130d3..fd7a0b9 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 d3594d3..47af20a 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. */ -- 1.8.3.1