From: Richard W.M. Jones Date: Thu, 2 Dec 2010 13:32:40 +0000 (+0000) Subject: generator: Code to handle optional arguments in daemon functions. X-Git-Tag: 1.7.19~6 X-Git-Url: http://git.annexia.org/?a=commitdiff_plain;h=65f44b459070a1dbfba66c31e0be69588e49f4a8;p=libguestfs.git generator: Code to handle optional arguments in daemon functions. Previously we only supported optional arguments for library functions (commit 14490c3e1aac61c6ac90f28828896683f64f0dc9). This extends that work so that optional arguments can also be passed through to the daemon. --- diff --git a/daemon/proto.c b/daemon/proto.c index 1fdb26c..00e3166 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 24f4e2c..0b3c850 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 f5be3e7..5b40cc2 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 e3d87e5..f15ae61 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 ca114c5..5714c80 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 e4e198b..bb68298 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 a5d9d2b..0d63af6 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"));