diff options
| -rw-r--r-- | daemon/proto.c | 8 | ||||
| -rw-r--r-- | generator/generator_c.ml | 108 | ||||
| -rw-r--r-- | generator/generator_capitests.ml | 66 | ||||
| -rw-r--r-- | generator/generator_daemon.ml | 215 | ||||
| -rw-r--r-- | generator/generator_xdr.ml | 10 | ||||
| -rw-r--r-- | src/guestfs-internal.h | 2 | ||||
| -rw-r--r-- | src/proto.c | 5 |
7 files changed, 244 insertions, 170 deletions
diff --git a/daemon/proto.c b/daemon/proto.c index 1fdb26c7..00e3166c 100644 --- a/daemon/proto.c +++ b/daemon/proto.c @@ -167,14 +167,6 @@ main_loop (int _sock) reply_with_error ("unexpected message status (%d)", hdr.status); goto cont; } - /* This version of the daemon does not understand optional arguments - * at all. When we fix this, we will remove the next conditional. - */ - if (hdr.optargs_bitmask != 0) { - reply_with_error ("optargs_bitmask != 0 (%" PRIu64 ")", - hdr.optargs_bitmask); - goto cont; - } proc_nr = hdr.proc; serial = hdr.serial; diff --git a/generator/generator_c.ml b/generator/generator_c.ml index 24f4e2cb..0b3c8508 100644 --- a/generator/generator_c.ml +++ b/generator/generator_c.ml @@ -909,19 +909,17 @@ check_state (guestfs_h *g, const char *caller) (* Client-side stubs for each function. *) List.iter ( fun (shortname, (ret, args, optargs as style), _, _, _, _, _) -> - if optargs <> [] then - failwithf "optargs not yet implemented for daemon functions"; - let name = "guestfs_" ^ shortname in let error_code = error_code_of ret in (* Generate the action stub. *) if optargs = [] then generate_prototype ~extern:false ~semicolon:false ~newline:true - ~handle:"g" name style + ~handle:"g" ~prefix:"guestfs_" shortname style else generate_prototype ~extern:false ~semicolon:false ~newline:true - ~handle:"g" ~suffix:"_argv" ~optarg_proto:Argv name style; + ~handle:"g" ~prefix:"guestfs_" ~suffix:"_argv" + ~optarg_proto:Argv shortname style; pr "{\n"; @@ -999,45 +997,69 @@ check_state (guestfs_h *g, const char *caller) pr "\n"; (* Send the main header and arguments. *) - (match args with - | [] -> - pr " serial = guestfs___send (g, GUESTFS_PROC_%s, progress_hint,\n" - (String.uppercase shortname); - pr " NULL, NULL);\n" - | args -> - List.iter ( - function - | Pathname n | Device n | Dev_or_Path n | String n | Key n -> - pr " args.%s = (char *) %s;\n" n n - | OptString n -> - pr " args.%s = %s ? (char **) &%s : NULL;\n" n n n - | StringList n | DeviceList n -> - pr " args.%s.%s_val = (char **) %s;\n" n n n; - pr " for (args.%s.%s_len = 0; %s[args.%s.%s_len]; args.%s.%s_len++) ;\n" n n n n n n n; - | Bool n -> - pr " args.%s = %s;\n" n n - | Int n -> - pr " args.%s = %s;\n" n n + if args = [] && optargs = [] then ( + pr " serial = guestfs___send (g, GUESTFS_PROC_%s, progress_hint, 0,\n" + (String.uppercase shortname); + pr " NULL, NULL);\n" + ) else ( + List.iter ( + function + | Pathname n | Device n | Dev_or_Path n | String n | Key n -> + pr " args.%s = (char *) %s;\n" n n + | OptString n -> + pr " args.%s = %s ? (char **) &%s : NULL;\n" n n n + | StringList n | DeviceList n -> + pr " args.%s.%s_val = (char **) %s;\n" n n n; + pr " for (args.%s.%s_len = 0; %s[args.%s.%s_len]; args.%s.%s_len++) ;\n" n n n n n n n; + | Bool n -> + pr " args.%s = %s;\n" n n + | Int n -> + pr " args.%s = %s;\n" n n + | Int64 n -> + pr " args.%s = %s;\n" n n + | FileIn _ | FileOut _ -> () + | BufferIn n -> + pr " /* Just catch grossly large sizes. XDR encoding will make this precise. */\n"; + pr " if (%s_size >= GUESTFS_MESSAGE_MAX) {\n" n; + trace_return_error ~indent:4 style; + pr " error (g, \"%%s: size of input buffer too large\", \"%s\");\n" + shortname; + pr " guestfs___end_busy (g);\n"; + pr " return %s;\n" error_code; + pr " }\n"; + pr " args.%s.%s_val = (char *) %s;\n" n n n; + pr " args.%s.%s_len = %s_size;\n" n n n + | Pointer _ -> assert false + ) args; + + List.iter ( + fun argt -> + let n = name_of_argt argt in + let uc_shortname = String.uppercase shortname in + let uc_n = String.uppercase n in + pr " if ((optargs->bitmask & GUESTFS_%s_%s_BITMASK))\n" + uc_shortname uc_n; + (match argt with + | Bool n + | Int n | Int64 n -> - pr " args.%s = %s;\n" n n - | FileIn _ | FileOut _ -> () - | BufferIn n -> - pr " /* Just catch grossly large sizes. XDR encoding will make this precise. */\n"; - pr " if (%s_size >= GUESTFS_MESSAGE_MAX) {\n" n; - trace_return_error ~indent:4 style; - pr " error (g, \"%%s: size of input buffer too large\", \"%s\");\n" - shortname; - pr " guestfs___end_busy (g);\n"; - pr " return %s;\n" error_code; - pr " }\n"; - pr " args.%s.%s_val = (char *) %s;\n" n n n; - pr " args.%s.%s_len = %s_size;\n" n n n - | Pointer _ -> assert false - ) args; - pr " serial = guestfs___send (g, GUESTFS_PROC_%s, progress_hint,\n" - (String.uppercase shortname); - pr " (xdrproc_t) xdr_%s_args, (char *) &args);\n" - name; + pr " args.%s = optargs->%s;\n" n n; + pr " else\n"; + pr " args.%s = 0;\n" n + | String n -> + pr " args.%s = (char *) %s;\n" n n; + pr " else\n"; + pr " args.%s = (char *) \"\";\n" n + | _ -> assert false + ) + ) optargs; + + pr " serial = guestfs___send (g, GUESTFS_PROC_%s,\n" + (String.uppercase shortname); + pr " progress_hint, %s,\n" + (if optargs <> [] then "optargs->bitmask" else "0"); + pr " (xdrproc_t) xdr_%s_args, (char *) &args);\n" + name; ); pr " if (serial == -1) {\n"; pr " guestfs___end_busy (g);\n"; diff --git a/generator/generator_capitests.ml b/generator/generator_capitests.ml index f5be3e7c..5b40cc2e 100644 --- a/generator/generator_capitests.ml +++ b/generator/generator_capitests.ml @@ -709,7 +709,7 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | [] -> assert false | name :: args -> (* Look up the command to find out what args/ret it has. *) - let style = + let style_ret, style_args, style_optargs = try let _, style, _, _, _, _, _ = List.find (fun (n, _, _, _, _, _, _) -> n = name) all_functions in @@ -717,16 +717,24 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = with Not_found -> failwithf "%s: in test, command %s was not found" test_name name in - (* If the call has optional args, fold them all together. We cannot - * test partial optional args yet. - *) - let style = - let ret, args, optargs = style in - ret, args@optargs in - - if List.length (snd style) <> List.length args then - failwithf "%s: in test, wrong number of args given to %s" - test_name name; + (* Match up the arguments strings and argument types. *) + let args, optargs = + let rec loop argts args = + match argts, args with + | (t::ts), (s::ss) -> + let args, rest = loop ts ss in + ((t, s) :: args), rest + | [], ss -> [], ss + | ts, [] -> + failwithf "%s: in test, too few args given to function %s" + test_name name + in + let args, optargs = loop style_args args in + let optargs, rest = loop style_optargs optargs in + if rest <> [] then + failwithf "%s: in test, too many args given to function %s" + test_name name; + args, optargs in pr " {\n"; @@ -764,10 +772,28 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | Pointer _, _ -> (* Difficult to make these pointers in order to run a test. *) assert false - ) (List.combine (snd style) args); + ) args; + + (* Currently can only deal with a complete, in-order list of optargs. *) + if optargs <> [] then ( + pr " struct guestfs_%s_argv optargs;\n" name; + let len = List.length style_optargs in + let bitmask = Int64.pred (Int64.shift_left 1L len) in + pr " optargs.bitmask = UINT64_C(0x%Lx);\n" bitmask; + List.iter ( + function + | Bool n, arg + | Int n, arg + | Int64 n, arg -> + pr " optargs.%s = %s;\n" n arg + | String n, arg -> + pr " optargs.%s = \"%s\";\n" n (c_quote arg); + | _ -> assert false + ) optargs; + ); let error_code = - match fst style with + match style_ret with | RErr | RInt _ | RBool _ -> pr " int r;\n"; "-1" | RInt64 _ -> pr " int64_t r;\n"; "-1" | RConstString _ | RConstOptString _ -> @@ -787,7 +813,10 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = "NULL" in pr " suppress_error = %d;\n" (if expect_error then 1 else 0); - pr " r = guestfs_%s (g" name; + if optargs = [] then + pr " r = guestfs_%s (g" name + else + pr " r = guestfs_%s_argv (g" name; (* Generate the parameters. *) List.iter ( @@ -820,13 +849,16 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | Bool _, arg -> let b = bool_of_string arg in pr ", %d" (if b then 1 else 0) | Pointer _, _ -> assert false - ) (List.combine (snd style) args); + ) args; - (match fst style with + (match style_ret with | RBufferOut _ -> pr ", &size" | _ -> () ); + if optargs <> [] then + pr ", &optargs"; + pr ");\n"; if not expect_error then @@ -841,7 +873,7 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | Some f -> f () ); - (match fst style with + (match style_ret with | RErr | RInt _ | RInt64 _ | RBool _ | RConstString _ | RConstOptString _ -> () | RString _ | RBufferOut _ -> pr " free (r);\n" diff --git a/generator/generator_daemon.ml b/generator/generator_daemon.ml index e3d87e50..f15ae612 100644 --- a/generator/generator_daemon.ml +++ b/generator/generator_daemon.ml @@ -37,7 +37,22 @@ let generate_daemon_actions_h () = pr "\n"; List.iter ( - fun (name, style, _, _, _, _, _) -> + function + | shortname, (_, _, (_::_ as optargs)), _, _, _, _, _ -> + iteri ( + fun i arg -> + let uc_shortname = String.uppercase shortname in + let n = name_of_argt arg in + let uc_n = String.uppercase n in + pr "#define GUESTFS_%s_%s_BITMASK (UINT64_C(1)<<%d)\n" + uc_shortname uc_n i + ) optargs + | _ -> () + ) daemon_functions; + + List.iter ( + fun (name, (ret, args, optargs), _, _, _, _, _) -> + let style = ret, args @ optargs, [] in generate_prototype ~single_line:true ~newline:true ~in_daemon:true ~prefix:"do_" name style; @@ -64,9 +79,6 @@ and generate_daemon_actions () = List.iter ( fun (name, (ret, args, optargs), _, _, _, _, _) -> - if optargs <> [] then - failwithf "optional arguments not supported in the daemon yet"; - (* Generate server-side stubs. *) pr "static void %s_stub (XDR *xdr_in)\n" name; pr "{\n"; @@ -86,98 +98,108 @@ and generate_daemon_actions () = pr " char *r;\n"; "NULL" in - (match args with - | [] -> () - | args -> - pr " struct guestfs_%s_args args;\n" name; - List.iter ( - function - | Device n | Dev_or_Path n - | Pathname n - | String n - | Key n -> () - | OptString n -> pr " char *%s;\n" n - | StringList n | DeviceList n -> pr " char **%s;\n" n - | Bool n -> pr " int %s;\n" n - | Int n -> pr " int %s;\n" n - | Int64 n -> pr " int64_t %s;\n" n - | FileIn _ | FileOut _ -> () - | BufferIn n -> - pr " const char *%s;\n" n; - pr " size_t %s_size;\n" n - | Pointer _ -> assert false - ) args + if args <> [] || optargs <> [] then ( + pr " struct guestfs_%s_args args;\n" name; + List.iter ( + function + | Device n | Dev_or_Path n + | Pathname n + | String n + | Key n -> () + | OptString n -> pr " char *%s;\n" n + | StringList n | DeviceList n -> pr " char **%s;\n" n + | Bool n -> pr " int %s;\n" n + | Int n -> pr " int %s;\n" n + | Int64 n -> pr " int64_t %s;\n" n + | FileIn _ | FileOut _ -> () + | BufferIn n -> + pr " const char *%s;\n" n; + pr " size_t %s_size;\n" n + | Pointer _ -> assert false + ) (args @ optargs) ); pr "\n"; let is_filein = List.exists (function FileIn _ -> true | _ -> false) args in - (match args with - | [] -> () - | args -> - pr " memset (&args, 0, sizeof args);\n"; - pr "\n"; - pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name; - if is_filein then - pr " if (cancel_receive () != -2)\n"; - pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n"; - pr " goto done;\n"; - pr " }\n"; - let pr_args n = - pr " char *%s = args.%s;\n" n n - in - let pr_list_handling_code n = - pr " %s = realloc (args.%s.%s_val,\n" n n n; - pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n; - pr " if (%s == NULL) {\n" n; - if is_filein then - pr " if (cancel_receive () != -2)\n"; - pr " reply_with_perror (\"realloc\");\n"; - pr " goto done;\n"; - pr " }\n"; - pr " %s[args.%s.%s_len] = NULL;\n" n n n; - pr " args.%s.%s_val = %s;\n" n n n; - in - List.iter ( - function - | Pathname n -> - pr_args n; - pr " ABS_PATH (%s, %s, goto done);\n" - n (if is_filein then "cancel_receive ()" else "0"); - | Device n -> - pr_args n; - pr " RESOLVE_DEVICE (%s, %s, goto done);\n" - n (if is_filein then "cancel_receive ()" else "0"); - | Dev_or_Path n -> - pr_args n; - pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n" - n (if is_filein then "cancel_receive ()" else "0"); - | String n | Key n -> pr_args n - | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n - | StringList n -> - pr_list_handling_code n; - | DeviceList n -> - pr_list_handling_code n; - pr " /* Ensure that each is a device,\n"; - pr " * and perform device name translation.\n"; - pr " */\n"; - pr " {\n"; - pr " size_t i;\n"; - pr " for (i = 0; %s[i] != NULL; ++i)\n" n; - pr " RESOLVE_DEVICE (%s[i], %s, goto done);\n" n - (if is_filein then "cancel_receive ()" else "0"); - pr " }\n"; - | Bool n -> pr " %s = args.%s;\n" n n - | Int n -> pr " %s = args.%s;\n" n n - | Int64 n -> pr " %s = args.%s;\n" n n - | FileIn _ | FileOut _ -> () - | BufferIn n -> - pr " %s = args.%s.%s_val;\n" n n n; - pr " %s_size = args.%s.%s_len;\n" n n n - | Pointer _ -> assert false - ) args; - pr "\n" + (* Reject unknown optional arguments. *) + if optargs <> [] then ( + let len = List.length optargs in + let mask = Int64.lognot (Int64.pred (Int64.shift_left 1L len)) in + pr " if (optargs_bitmask & UINT64_C(0x%Lx)) {\n" mask; + if is_filein then + pr " if (cancel_receive () != -2)\n"; + pr " reply_with_error (\"unknown option in optional arguments bitmask (this can happen if a program is compiled against a newer version of libguestfs, then run against an older version of the daemon)\");\n"; + pr " goto done;\n"; + pr " }\n"; + pr "\n" + ); + + (* Decode arguments. *) + if args <> [] || optargs <> [] then ( + pr " memset (&args, 0, sizeof args);\n"; + pr "\n"; + pr " if (!xdr_guestfs_%s_args (xdr_in, &args)) {\n" name; + if is_filein then + pr " if (cancel_receive () != -2)\n"; + pr " reply_with_error (\"daemon failed to decode procedure arguments\");\n"; + pr " goto done;\n"; + pr " }\n"; + let pr_args n = + pr " char *%s = args.%s;\n" n n + in + let pr_list_handling_code n = + pr " %s = realloc (args.%s.%s_val,\n" n n n; + pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n; + pr " if (%s == NULL) {\n" n; + if is_filein then + pr " if (cancel_receive () != -2)\n"; + pr " reply_with_perror (\"realloc\");\n"; + pr " goto done;\n"; + pr " }\n"; + pr " %s[args.%s.%s_len] = NULL;\n" n n n; + pr " args.%s.%s_val = %s;\n" n n n; + in + List.iter ( + function + | Pathname n -> + pr_args n; + pr " ABS_PATH (%s, %s, goto done);\n" + n (if is_filein then "cancel_receive ()" else "0"); + | Device n -> + pr_args n; + pr " RESOLVE_DEVICE (%s, %s, goto done);\n" + n (if is_filein then "cancel_receive ()" else "0"); + | Dev_or_Path n -> + pr_args n; + pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, %s, goto done);\n" + n (if is_filein then "cancel_receive ()" else "0"); + | String n | Key n -> pr_args n + | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n + | StringList n -> + pr_list_handling_code n; + | DeviceList n -> + pr_list_handling_code n; + pr " /* Ensure that each is a device,\n"; + pr " * and perform device name translation.\n"; + pr " */\n"; + pr " {\n"; + pr " size_t i;\n"; + pr " for (i = 0; %s[i] != NULL; ++i)\n" n; + pr " RESOLVE_DEVICE (%s[i], %s, goto done);\n" n + (if is_filein then "cancel_receive ()" else "0"); + pr " }\n"; + | Bool n -> pr " %s = args.%s;\n" n n + | Int n -> pr " %s = args.%s;\n" n n + | Int64 n -> pr " %s = args.%s;\n" n n + | FileIn _ | FileOut _ -> () + | BufferIn n -> + pr " %s = args.%s.%s_val;\n" n n n; + pr " %s_size = args.%s.%s_len;\n" n n n + | Pointer _ -> assert false + ) (args @ optargs); + pr "\n" ); (* this is used at least for do_equal *) @@ -191,11 +213,14 @@ and generate_daemon_actions () = (* Don't want to call the impl with any FileIn or FileOut * parameters, since these go "outside" the RPC protocol. *) - let args' = - List.filter (function FileIn _ | FileOut _ -> false | _ -> true) args in - pr " r = do_%s " name; - generate_c_call_args (ret, args', optargs); - pr ";\n"; + let () = + let args' = + List.filter + (function FileIn _ | FileOut _ -> false | _ -> true) args in + let style = ret, args' @ optargs, [] in + pr " r = do_%s " name; + generate_c_call_args style; + pr ";\n" in (match ret with | RErr | RInt _ | RInt64 _ | RBool _ diff --git a/generator/generator_xdr.ml b/generator/generator_xdr.ml index ca114c59..5714c803 100644 --- a/generator/generator_xdr.ml +++ b/generator/generator_xdr.ml @@ -65,12 +65,14 @@ let generate_xdr () = List.iter ( fun (shortname, (ret, args, optargs), _, _, _, _, _) -> - if optargs <> [] then - failwithf "optional arguments not supported in XDR yet"; - let name = "guestfs_" ^ shortname in - (match args with + (* Ordinary arguments and optional arguments are concatenated + * together in the XDR args struct. The optargs_bitmask field + * in the header controls which optional arguments are + * meaningful. + *) + (match args @ optargs with | [] -> () | args -> pr "struct %s_args {\n" name; diff --git a/src/guestfs-internal.h b/src/guestfs-internal.h index e4e198b1..bb682984 100644 --- a/src/guestfs-internal.h +++ b/src/guestfs-internal.h @@ -250,7 +250,7 @@ extern void guestfs___print_timestamped_message (guestfs_h *g, const char *fs, . extern void guestfs___free_inspect_info (guestfs_h *g); extern int guestfs___set_busy (guestfs_h *g); extern int guestfs___end_busy (guestfs_h *g); -extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, xdrproc_t xdrp, char *args); +extern int guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, uint64_t optargs_bitmask, xdrproc_t xdrp, char *args); extern int guestfs___recv (guestfs_h *g, const char *fn, struct guestfs_message_header *hdr, struct guestfs_message_error *err, xdrproc_t xdrp, char *ret); extern int guestfs___send_file (guestfs_h *g, const char *filename); extern int guestfs___recv_file (guestfs_h *g, const char *filename); diff --git a/src/proto.c b/src/proto.c index a5d9d2b8..0d63af60 100644 --- a/src/proto.c +++ b/src/proto.c @@ -693,7 +693,8 @@ guestfs___accept_from_daemon (guestfs_h *g) } int -guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, +guestfs___send (guestfs_h *g, int proc_nr, + uint64_t progress_hint, uint64_t optargs_bitmask, xdrproc_t xdrp, char *args) { struct guestfs_message_header hdr; @@ -726,7 +727,7 @@ guestfs___send (guestfs_h *g, int proc_nr, uint64_t progress_hint, hdr.serial = serial; hdr.status = GUESTFS_STATUS_OK; hdr.progress_hint = progress_hint; - hdr.optargs_bitmask = 0; + hdr.optargs_bitmask = optargs_bitmask; if (!xdr_guestfs_message_header (&xdr, &hdr)) { error (g, _("xdr_guestfs_message_header failed")); |