Richard W.M. Jones
2011-Mar-07 19:34 UTC
[Libguestfs] [PATCH] generator: Introduce error code (errcode) concept.
I have pushed this, this is just FYI. See the end of the patch for the new type and functions introduced into the generator. Passes all the tests, but should still be treated with a bit of suspicion. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org -------------- next part -------------->From 8037da06feea097716ce700f38c0eac0d5411a7c Mon Sep 17 00:00:00 2001From: Richard W.M. Jones <rjones at redhat.com> Date: Mon, 7 Mar 2011 19:28:30 +0000 Subject: [PATCH] generator: Introduce error code (errcode) concept. There was a lot of repeated code to map return types (eg. RErr) to error cases (eg. -1 or NULL). This commit introduces an error code type and two functions to map return types to error codes and error codes to strings. --- generator/generator_c.ml | 122 ++++++++++++++++++++++---------------- generator/generator_capitests.ml | 56 ++++++++++-------- generator/generator_daemon.ml | 40 +++++++------ generator/generator_java.ml | 85 +++++++++++++++------------ generator/generator_ocaml.ml | 58 ++++++++++-------- generator/generator_php.ml | 57 ++++++++++-------- generator/generator_python.ml | 46 ++++++++------ generator/generator_ruby.ml | 40 +++++++----- generator/generator_types.ml | 4 +- generator/generator_utils.ml | 17 +++++- generator/generator_utils.mli | 13 ++++- 11 files changed, 315 insertions(+), 223 deletions(-) diff --git a/generator/generator_c.ml b/generator/generator_c.ml index 99cf327..9b88376 100644 --- a/generator/generator_c.ml +++ b/generator/generator_c.ml @@ -637,14 +637,6 @@ check_state (guestfs_h *g, const char *caller) "; - let error_code_of = function - | RErr | RInt _ | RInt64 _ | RBool _ -> "-1" - | RConstString _ | RConstOptString _ - | RString _ | RStringList _ - | RStruct _ | RStructList _ - | RHashtable _ | RBufferOut _ -> "NULL" - in - (* Generate code to check String-like parameters are not passed in * as NULL (returning an error if they are). *) @@ -667,7 +659,17 @@ check_state (guestfs_h *g, const char *caller) pr " if (%s == NULL) {\n" n; pr " error (g, \"%%s: %%s: parameter cannot be NULL\",\n"; pr " \"%s\", \"%s\");\n" shortname n; - pr " return %s;\n" (error_code_of ret); + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> + if shortname = "test0rconstoptstring" then (* XXX hack *) + `ErrorIsNULL + else + failwithf + "%s: RConstOptString function has invalid parameter '%s'" + shortname n + | (`ErrorIsMinusOne |`ErrorIsNULL) as e -> e in + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr_newline := true @@ -689,7 +691,11 @@ check_state (guestfs_h *g, const char *caller) pr " optargs->%s == NULL) {\n" n; pr " error (g, \"%%s: %%s: optional parameter cannot be NULL\",\n"; pr " \"%s\", \"%s\");\n" shortname n; - pr " return %s;\n" (error_code_of ret); + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> assert false + | (`ErrorIsMinusOne |`ErrorIsNULL) as e -> e in + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr_newline := true @@ -711,7 +717,11 @@ check_state (guestfs_h *g, const char *caller) pr " if (optargs->bitmask & UINT64_C(0x%Lx)) {\n" mask; pr " error (g, \"%%s: unknown option in guestfs_%%s_argv->bitmask (this can happen if a program is compiled against a newer version of libguestfs, then dynamically linked to an older version)\",\n"; pr " \"%s\", \"%s\");\n" shortname shortname; - pr " return %s;\n" (error_code_of ret); + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> assert false + | (`ErrorIsMinusOne |`ErrorIsNULL) as e -> e in + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; in @@ -887,24 +897,24 @@ check_state (guestfs_h *g, const char *caller) shortname style; pr "{\n"; pr " int trace_flag = g->trace;\n"; - let errcode - match ret with - | RErr | RInt _ | RBool _ -> - pr " int r;\n"; Some "-1" - | RInt64 _ -> - pr " int64_t r;\n"; Some "-1" - | RConstString _ -> - pr " const char *r;\n"; Some "NULL" - | RConstOptString _ -> - pr " const char *r;\n"; None - | RString _ | RBufferOut _ -> - pr " char *r;\n"; Some "NULL" - | RStringList _ | RHashtable _ -> - pr " char **r;\n"; Some "NULL" - | RStruct (_, typ) -> - pr " struct guestfs_%s *r;\n" typ; Some "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; Some "NULL" in + (match ret with + | RErr | RInt _ | RBool _ -> + pr " int r;\n" + | RInt64 _ -> + pr " int64_t r;\n" + | RConstString _ -> + pr " const char *r;\n" + | RConstOptString _ -> + pr " const char *r;\n" + | RString _ | RBufferOut _ -> + pr " char *r;\n" + | RStringList _ | RHashtable _ -> + pr " char **r;\n" + | RStruct (_, typ) -> + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + ); pr "\n"; check_null_strings shortname style; reject_unknown_optargs shortname style; @@ -913,14 +923,14 @@ check_state (guestfs_h *g, const char *caller) generate_c_call_args ~handle:"g" style; pr ";\n"; pr "\n"; - (match errcode with - | Some errcode -> - pr " if (r != %s) {\n" errcode; + (match errcode_of_ret ret with + | (`ErrorIsMinusOne | `ErrorIsNULL) as errcode -> + pr " if (r != %s) {\n" (string_of_errcode errcode); trace_return ~indent:4 shortname style "r"; pr " } else {\n"; trace_return_error ~indent:4 shortname style; pr " }\n"; - | None -> + | `CannotReturnError -> trace_return shortname style "r"; ); pr "\n"; @@ -933,7 +943,10 @@ check_state (guestfs_h *g, const char *caller) List.iter ( fun (shortname, (ret, args, optargs as style), _, _, _, _, _) -> let name = "guestfs_" ^ shortname in - let error_code = error_code_of ret in + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> assert false + | (`ErrorIsMinusOne | `ErrorIsNULL) as e -> e in (* Generate the action stub. *) if optargs = [] then @@ -1014,7 +1027,7 @@ check_state (guestfs_h *g, const char *caller) (* Check we are in the right state for sending a request. *) pr " if (check_state (g, \"%s\") == -1) {\n" shortname; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr " guestfs___set_busy (g);\n"; pr "\n"; @@ -1048,7 +1061,7 @@ check_state (guestfs_h *g, const char *caller) 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 " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr " args.%s.%s_val = (char *) %s;\n" n n n; pr " args.%s.%s_len = %s_size;\n" n n n @@ -1087,7 +1100,7 @@ check_state (guestfs_h *g, const char *caller) pr " if (serial == -1) {\n"; pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; @@ -1100,7 +1113,7 @@ check_state (guestfs_h *g, const char *caller) pr " if (r == -1) {\n"; pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr " if (r == -2) /* daemon cancelled */\n"; pr " goto read_reply;\n"; @@ -1125,7 +1138,7 @@ check_state (guestfs_h *g, const char *caller) pr " if (r == -1) {\n"; pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; @@ -1133,7 +1146,7 @@ check_state (guestfs_h *g, const char *caller) (String.uppercase shortname); pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; @@ -1152,7 +1165,7 @@ check_state (guestfs_h *g, const char *caller) pr " free (err.error_message);\n"; pr " free (err.errno_string);\n"; pr " guestfs___end_busy (g);\n"; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; @@ -1163,7 +1176,7 @@ check_state (guestfs_h *g, const char *caller) pr " if (guestfs___recv_file (g, %s) == -1) {\n" n; pr " guestfs___end_busy (g);\n"; trace_return_error ~indent:4 shortname style; - pr " return %s;\n" error_code; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; | _ -> () @@ -1254,16 +1267,16 @@ check_state (guestfs_h *g, const char *caller) | [] -> "g" | args -> name_of_argt (List.hd (List.rev args)) in - let rerrcode, rtype + let rtype match ret with - | RErr | RInt _ | RBool _ -> "-1", "int " - | RInt64 _ -> "-1", "int64_t " - | RConstString _ | RConstOptString _ -> "NULL", "const char *" - | RString _ | RBufferOut _ -> "NULL", "char *" - | RStringList _ | RHashtable _ -> "NULL", "char **" - | RStruct (_, typ) -> "NULL", sprintf "struct guestfs_%s *" typ + | RErr | RInt _ | RBool _ -> "int " + | RInt64 _ -> "int64_t " + | RConstString _ | RConstOptString _ -> "const char *" + | RString _ | RBufferOut _ -> "char *" + | RStringList _ | RHashtable _ -> "char **" + | RStruct (_, typ) -> sprintf "struct guestfs_%s *" typ | RStructList (_, typ) -> - "NULL", sprintf "struct guestfs_%s_list *" typ in + sprintf "struct guestfs_%s_list *" typ in (* The regular variable args function, just calls the _va variant. *) generate_prototype ~extern:false ~semicolon:false ~newline:true @@ -1309,17 +1322,22 @@ check_state (guestfs_h *g, const char *caller) pr " break;\n"; ) optargs; + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> assert false + | (`ErrorIsMinusOne | `ErrorIsNULL) as e -> e in + pr " default:\n"; pr " error (g, \"%%s: unknown option %%d (this can happen if a program is compiled against a newer version of libguestfs, then dynamically linked to an older version)\",\n"; pr " \"%s\", i);\n" shortname; - pr " return %s;\n" rerrcode; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr "\n"; pr " uint64_t i_mask = UINT64_C(1) << i;\n"; pr " if (optargs_s.bitmask & i_mask) {\n"; pr " error (g, \"%%s: same optional argument specified more than once\",\n"; pr " \"%s\");\n" shortname; - pr " return %s;\n" rerrcode; + pr " return %s;\n" (string_of_errcode errcode); pr " }\n"; pr " optargs_s.bitmask |= i_mask;\n"; pr " }\n"; diff --git a/generator/generator_capitests.ml b/generator/generator_capitests.ml index 5b40cc2..196b1fb 100644 --- a/generator/generator_capitests.ml +++ b/generator/generator_capitests.ml @@ -792,25 +792,23 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd ) optargs; ); - let error_code - match style_ret with - | RErr | RInt _ | RBool _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RConstString _ | RConstOptString _ -> - pr " const char *r;\n"; "NULL" - | RString _ -> pr " char *r;\n"; "NULL" - | RStringList _ | RHashtable _ -> - pr " char **r;\n"; - pr " size_t i;\n"; - "NULL" - | RStruct (_, typ) -> - pr " struct guestfs_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; "NULL" - | RBufferOut _ -> - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL" in + (match style_ret with + | RErr | RInt _ | RBool _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RConstString _ | RConstOptString _ -> + pr " const char *r;\n" + | RString _ -> pr " char *r;\n" + | RStringList _ | RHashtable _ -> + pr " char **r;\n"; + pr " size_t i;\n" + | RStruct (_, typ) -> + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + | RBufferOut _ -> + pr " char *r;\n"; + pr " size_t size;\n" + ); pr " suppress_error = %d;\n" (if expect_error then 1 else 0); if optargs = [] then @@ -861,11 +859,21 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd pr ");\n"; - if not expect_error then - pr " if (r == %s)\n" error_code - else - pr " if (r != %s)\n" error_code; - pr " return -1;\n"; + (match errcode_of_ret style_ret, expect_error with + | `CannotReturnError, _ -> () + | `ErrorIsMinusOne, false -> + pr " if (r == -1)\n"; + pr " return -1;\n"; + | `ErrorIsMinusOne, true -> + pr " if (r != -1)\n"; + pr " return -1;\n"; + | `ErrorIsNULL, false -> + pr " if (r == NULL)\n"; + pr " return -1;\n"; + | `ErrorIsNULL, true -> + pr " if (r != NULL)\n"; + pr " return -1;\n"; + ); (* Insert the test code. *) (match test with diff --git a/generator/generator_daemon.ml b/generator/generator_daemon.ml index f15ae61..21a5888 100644 --- a/generator/generator_daemon.ml +++ b/generator/generator_daemon.ml @@ -82,21 +82,20 @@ and generate_daemon_actions () (* Generate server-side stubs. *) pr "static void %s_stub (XDR *xdr_in)\n" name; pr "{\n"; - let error_code - match ret with - | RErr | RInt _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RBool _ -> pr " int r;\n"; "-1" - | RConstString _ | RConstOptString _ -> - failwithf "RConstString|RConstOptString cannot be used by daemon functions" - | RString _ -> pr " char *r;\n"; "NULL" - | RStringList _ | RHashtable _ -> pr " char **r;\n"; "NULL" - | RStruct (_, typ) -> pr " guestfs_int_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> pr " guestfs_int_%s_list *r;\n" typ; "NULL" - | RBufferOut _ -> - pr " size_t size = 1;\n"; - pr " char *r;\n"; - "NULL" in + (match ret with + | RErr | RInt _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RBool _ -> pr " int r;\n" + | RConstString _ | RConstOptString _ -> + failwithf "RConstString|RConstOptString cannot be used by daemon functions" + | RString _ -> pr " char *r;\n" + | RStringList _ | RHashtable _ -> pr " char **r;\n" + | RStruct (_, typ) -> pr " guestfs_int_%s *r;\n" typ + | RStructList (_, typ) -> pr " guestfs_int_%s_list *r;\n" typ + | RBufferOut _ -> + pr " size_t size = 1;\n"; + pr " char *r;\n" + ); if args <> [] || optargs <> [] then ( pr " struct guestfs_%s_args args;\n" name; @@ -223,11 +222,16 @@ and generate_daemon_actions () pr ";\n" in (match ret with + | RConstOptString _ -> assert false | RErr | RInt _ | RInt64 _ | RBool _ - | RConstString _ | RConstOptString _ + | RConstString _ | RString _ | RStringList _ | RHashtable _ | RStruct (_, _) | RStructList (_, _) -> - pr " if (r == %s)\n" error_code; + let errcode + match errcode_of_ret ret with + | `CannotReturnError -> assert false + | (`ErrorIsMinusOne | `ErrorIsNULL) as e -> e in + pr " if (r == %s)\n" (string_of_errcode errcode); pr " /* do_%s has already called reply_with_error */\n" name; pr " goto done;\n"; pr "\n" @@ -235,7 +239,7 @@ and generate_daemon_actions () pr " /* size == 0 && r == NULL could be a non-error case (just\n"; pr " * an ordinary zero-length buffer), so be careful ...\n"; pr " */\n"; - pr " if (size == 1 && r == %s)\n" error_code; + pr " if (size == 1 && r == NULL)\n"; pr " /* do_%s has already called reply_with_error */\n" name; pr " goto done;\n"; pr "\n" diff --git a/generator/generator_java.ml b/generator/generator_java.ml index dedf962..767c94c 100644 --- a/generator/generator_java.ml +++ b/generator/generator_java.ml @@ -374,40 +374,40 @@ Java_com_redhat_et_libguestfs_GuestFS__1close pr ")\n"; pr "{\n"; pr " guestfs_h *g = (guestfs_h *) (long) jg;\n"; - let error_code, no_ret - match ret with - | RErr -> pr " int r;\n"; "-1", "" - | RBool _ - | RInt _ -> pr " int r;\n"; "-1", "0" - | RInt64 _ -> pr " int64_t r;\n"; "-1", "0" - | RConstString _ -> pr " const char *r;\n"; "NULL", "NULL" - | RConstOptString _ -> pr " const char *r;\n"; "NULL", "NULL" - | RString _ -> - pr " jstring jr;\n"; - pr " char *r;\n"; "NULL", "NULL" - | RStringList _ - | RHashtable _ -> - pr " jobjectArray jr;\n"; - pr " int r_len;\n"; - pr " jclass cl;\n"; - pr " jstring jstr;\n"; - pr " char **r;\n"; "NULL", "NULL" - | RStruct (_, typ) -> - pr " jobject jr;\n"; - pr " jclass cl;\n"; - pr " jfieldID fl;\n"; - pr " struct guestfs_%s *r;\n" typ; "NULL", "NULL" - | RStructList (_, typ) -> - pr " jobjectArray jr;\n"; - pr " jclass cl;\n"; - pr " jfieldID fl;\n"; - pr " jobject jfl;\n"; - pr " struct guestfs_%s_list *r;\n" typ; "NULL", "NULL" - | RBufferOut _ -> - pr " jstring jr;\n"; - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL", "NULL" in + (match ret with + | RErr -> pr " int r;\n" + | RBool _ + | RInt _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RConstString _ -> pr " const char *r;\n" + | RConstOptString _ -> pr " const char *r;\n" + | RString _ -> + pr " jstring jr;\n"; + pr " char *r;\n" + | RStringList _ + | RHashtable _ -> + pr " jobjectArray jr;\n"; + pr " int r_len;\n"; + pr " jclass cl;\n"; + pr " jstring jstr;\n"; + pr " char **r;\n" + | RStruct (_, typ) -> + pr " jobject jr;\n"; + pr " jclass cl;\n"; + pr " jfieldID fl;\n"; + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " jobjectArray jr;\n"; + pr " jclass cl;\n"; + pr " jfieldID fl;\n"; + pr " jobject jfl;\n"; + pr " struct guestfs_%s_list *r;\n" typ + | RBufferOut _ -> + pr " jstring jr;\n"; + pr " char *r;\n"; + pr " size_t size;\n" + ); + List.iter ( function | Pathname n @@ -527,10 +527,19 @@ Java_com_redhat_et_libguestfs_GuestFS__1close ) args; (* Check for errors. *) - pr " if (r == %s) {\n" error_code; - pr " throw_exception (env, guestfs_last_error (g));\n"; - pr " return %s;\n" no_ret; - pr " }\n"; + (match errcode_of_ret ret with + | `CannotReturnError -> () + | `ErrorIsMinusOne -> + pr " if (r == -1) {\n"; + pr " throw_exception (env, guestfs_last_error (g));\n"; + pr " return -1;\n"; + pr " }\n" + | `ErrorIsNULL -> + pr " if (r == NULL) {\n"; + pr " throw_exception (env, guestfs_last_error (g));\n"; + pr " return NULL;\n"; + pr " }\n" + ); (* Return value. *) (match ret with diff --git a/generator/generator_ocaml.ml b/generator/generator_ocaml.ml index 393a062..ced6fb4 100644 --- a/generator/generator_ocaml.ml +++ b/generator/generator_ocaml.ml @@ -430,31 +430,28 @@ copy_table (char * const * argv) ) optargs ); - let error_code - match ret with - | RErr -> pr " int r;\n"; "-1" - | RInt _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RBool _ -> pr " int r;\n"; "-1" - | RConstString _ | RConstOptString _ -> - pr " const char *r;\n"; "NULL" - | RString _ -> pr " char *r;\n"; "NULL" - | RStringList _ -> - pr " size_t i;\n"; - pr " char **r;\n"; - "NULL" - | RStruct (_, typ) -> - pr " struct guestfs_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; "NULL" - | RHashtable _ -> - pr " size_t i;\n"; - pr " char **r;\n"; - "NULL" - | RBufferOut _ -> - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL" in + (match ret with + | RErr -> pr " int r;\n" + | RInt _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RBool _ -> pr " int r;\n" + | RConstString _ | RConstOptString _ -> + pr " const char *r;\n" + | RString _ -> pr " char *r;\n" + | RStringList _ -> + pr " size_t i;\n"; + pr " char **r;\n" + | RStruct (_, typ) -> + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + | RHashtable _ -> + pr " size_t i;\n"; + pr " char **r;\n" + | RBufferOut _ -> + pr " char *r;\n"; + pr " size_t size;\n" + ); pr "\n"; pr " caml_enter_blocking_section ();\n"; @@ -487,8 +484,15 @@ copy_table (char * const * argv) | StringList _ | DeviceList _ | Pointer _ -> () ) optargs; - pr " if (r == %s)\n" error_code; - pr " ocaml_guestfs_raise_error (g, \"%s\");\n" name; + (match errcode_of_ret ret with + | `CannotReturnError -> () + | `ErrorIsMinusOne -> + pr " if (r == -1)\n"; + pr " ocaml_guestfs_raise_error (g, \"%s\");\n" name; + | `ErrorIsNULL -> + pr " if (r == NULL)\n"; + pr " ocaml_guestfs_raise_error (g, \"%s\");\n" name; + ); pr "\n"; (match ret with diff --git a/generator/generator_php.ml b/generator/generator_php.ml index d405656..4431147 100644 --- a/generator/generator_php.ml +++ b/generator/generator_php.ml @@ -357,28 +357,27 @@ PHP_FUNCTION (guestfs_last_error) ); (* Return value. *) - let error_code - match ret with - | RErr -> pr " int r;\n"; "-1" - | RBool _ - | RInt _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RConstString _ -> pr " const char *r;\n"; "NULL" - | RConstOptString _ -> pr " const char *r;\n"; "NULL" - | RString _ -> - pr " char *r;\n"; "NULL" - | RStringList _ -> - pr " char **r;\n"; "NULL" - | RStruct (_, typ) -> - pr " struct guestfs_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; "NULL" - | RHashtable _ -> - pr " char **r;\n"; "NULL" - | RBufferOut _ -> - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL" in + (match ret with + | RErr -> pr " int r;\n" + | RBool _ + | RInt _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RConstString _ -> pr " const char *r;\n" + | RConstOptString _ -> pr " const char *r;\n" + | RString _ -> + pr " char *r;\n" + | RStringList _ -> + pr " char **r;\n" + | RStruct (_, typ) -> + pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + | RHashtable _ -> + pr " char **r;\n" + | RBufferOut _ -> + pr " char *r;\n"; + pr " size_t size;\n" + ); (* Call the function. *) if optargs = [] then @@ -410,9 +409,17 @@ PHP_FUNCTION (guestfs_last_error) ) args; (* Check for errors. *) - pr " if (r == %s) {\n" error_code; - pr " RETURN_FALSE;\n"; - pr " }\n"; + (match errcode_of_ret ret with + | `CannotReturnError -> () + | `ErrorIsMinusOne -> + pr " if (r == -1) {\n"; + pr " RETURN_FALSE;\n"; + pr " }\n" + | `ErrorIsNULL -> + pr " if (r == NULL) {\n"; + pr " RETURN_FALSE;\n"; + pr " }\n" + ); pr "\n"; (* Convert the return value. *) diff --git a/generator/generator_python.ml b/generator/generator_python.ml index bc570a8..937c092 100644 --- a/generator/generator_python.ml +++ b/generator/generator_python.ml @@ -293,21 +293,20 @@ py_guestfs_close (PyObject *self, PyObject *args) pr " struct guestfs_%s_argv *optargs = &optargs_s;\n" name; ); - let error_code - match ret with - | RErr | RInt _ | RBool _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RConstString _ | RConstOptString _ -> - pr " const char *r;\n"; "NULL" - | RString _ -> pr " char *r;\n"; "NULL" - | RStringList _ | RHashtable _ -> pr " char **r;\n"; "NULL" - | RStruct (_, typ) -> pr " struct guestfs_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; "NULL" - | RBufferOut _ -> - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL" in + (match ret with + | RErr | RInt _ | RBool _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RConstString _ | RConstOptString _ -> + pr " const char *r;\n" + | RString _ -> pr " char *r;\n" + | RStringList _ | RHashtable _ -> pr " char **r;\n" + | RStruct (_, typ) -> pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + | RBufferOut _ -> + pr " char *r;\n"; + pr " size_t size;\n" + ); List.iter ( function @@ -457,10 +456,19 @@ py_guestfs_close (PyObject *self, PyObject *args) pr " free (%s);\n" n ) args; - pr " if (r == %s) {\n" error_code; - pr " PyErr_SetString (PyExc_RuntimeError, guestfs_last_error (g));\n"; - pr " return NULL;\n"; - pr " }\n"; + (match errcode_of_ret ret with + | `CannotReturnError -> () + | `ErrorIsMinusOne -> + pr " if (r == -1) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError, guestfs_last_error (g));\n"; + pr " return NULL;\n"; + pr " }\n" + | `ErrorIsNULL -> + pr " if (r == NULL) {\n"; + pr " PyErr_SetString (PyExc_RuntimeError, guestfs_last_error (g));\n"; + pr " return NULL;\n"; + pr " }\n" + ); pr "\n"; (match ret with diff --git a/generator/generator_ruby.ml b/generator/generator_ruby.ml index 6d627b0..49ddf1f 100644 --- a/generator/generator_ruby.ml +++ b/generator/generator_ruby.ml @@ -184,21 +184,20 @@ static VALUE ruby_guestfs_close (VALUE gv) pr "\n"; ); - let error_code - match ret with - | RErr | RInt _ | RBool _ -> pr " int r;\n"; "-1" - | RInt64 _ -> pr " int64_t r;\n"; "-1" - | RConstString _ | RConstOptString _ -> - pr " const char *r;\n"; "NULL" - | RString _ -> pr " char *r;\n"; "NULL" - | RStringList _ | RHashtable _ -> pr " char **r;\n"; "NULL" - | RStruct (_, typ) -> pr " struct guestfs_%s *r;\n" typ; "NULL" - | RStructList (_, typ) -> - pr " struct guestfs_%s_list *r;\n" typ; "NULL" - | RBufferOut _ -> - pr " char *r;\n"; - pr " size_t size;\n"; - "NULL" in + (match ret with + | RErr | RInt _ | RBool _ -> pr " int r;\n" + | RInt64 _ -> pr " int64_t r;\n" + | RConstString _ | RConstOptString _ -> + pr " const char *r;\n" + | RString _ -> pr " char *r;\n" + | RStringList _ | RHashtable _ -> pr " char **r;\n" + | RStruct (_, typ) -> pr " struct guestfs_%s *r;\n" typ + | RStructList (_, typ) -> + pr " struct guestfs_%s_list *r;\n" typ + | RBufferOut _ -> + pr " char *r;\n"; + pr " size_t size;\n" + ); pr "\n"; if optargs = [] then @@ -217,8 +216,15 @@ static VALUE ruby_guestfs_close (VALUE gv) pr " free (%s);\n" n ) args; - pr " if (r == %s)\n" error_code; - pr " rb_raise (e_Error, \"%%s\", guestfs_last_error (g));\n"; + (match errcode_of_ret ret with + | `CannotReturnError -> () + | `ErrorIsMinusOne -> + pr " if (r == -1)\n"; + pr " rb_raise (e_Error, \"%%s\", guestfs_last_error (g));\n" + | `ErrorIsNULL -> + pr " if (r == NULL)\n"; + pr " rb_raise (e_Error, \"%%s\", guestfs_last_error (g));\n" + ); pr "\n"; (match ret with diff --git a/generator/generator_types.ml b/generator/generator_types.ml index b6c2730..50e9604 100644 --- a/generator/generator_types.ml +++ b/generator/generator_types.ml @@ -1,5 +1,5 @@ (* libguestfs - * Copyright (C) 2009-2010 Red Hat Inc. + * Copyright (C) 2009-2011 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -203,6 +203,8 @@ and argt *) | Pointer of (string * string) +type errcode = [ `CannotReturnError | `ErrorIsMinusOne | `ErrorIsNULL ] + type flags | ProtocolLimitWarning (* display warning about protocol size limits *) | DangerWillRobinson (* flags particularly dangerous commands *) diff --git a/generator/generator_utils.ml b/generator/generator_utils.ml index 27b543d..f62b195 100644 --- a/generator/generator_utils.ml +++ b/generator/generator_utils.ml @@ -1,5 +1,5 @@ (* libguestfs - * Copyright (C) 2009-2010 Red Hat Inc. + * Copyright (C) 2009-2011 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -28,6 +28,21 @@ open Printf open Generator_types +let errcode_of_ret = function + | RConstOptString _ -> + `CannotReturnError + | RErr | RInt _ | RBool _ | RInt64 _ -> + `ErrorIsMinusOne + | RConstString _ + | RString _ | RBufferOut _ + | RStringList _ | RHashtable _ + | RStruct _ | RStructList _ -> + `ErrorIsNULL + +let string_of_errcode = function + | `ErrorIsMinusOne -> "-1" + | `ErrorIsNULL -> "NULL" + (* Generate a uuidgen-compatible UUID (used in tests). However to * avoid having the UUID change every time we rebuild the tests, * generate it as a function of the contents of the diff --git a/generator/generator_utils.mli b/generator/generator_utils.mli index 5104a16..5dc4da2 100644 --- a/generator/generator_utils.mli +++ b/generator/generator_utils.mli @@ -1,5 +1,5 @@ (* libguestfs - * Copyright (C) 2009-2010 Red Hat Inc. + * Copyright (C) 2009-2011 Red Hat Inc. * * This program is free software; you can redistribute it and/or modify * it under the terms of the GNU General Public License as published by @@ -20,6 +20,17 @@ (** Useful utility functions. *) +val errcode_of_ret : Generator_types.ret -> Generator_types.errcode +(** Map [ret] type to the error indication that the action returns, + eg. [errcode_of_ret RErr] => [`ErrorIsMinusOne] (meaning that + these actions return [-1]). + + Note that [RConstOptString] cannot return an error indication, and + this returns [`CannotReturnError]. Callers must deal with it. *) + +val string_of_errcode : [`ErrorIsMinusOne|`ErrorIsNULL] -> string +(** Return errcode as a string. Untyped for [`CannotReturnError]. *) + val uuidgen : unit -> string (** Generate a random UUID (used in tests). *) -- 1.7.4
Matthew Booth
2011-Mar-08 10:45 UTC
[Libguestfs] [PATCH] generator: Introduce error code (errcode) concept.
On 07/03/11 19:34, Richard W.M. Jones wrote:>> From 8037da06feea097716ce700f38c0eac0d5411a7c Mon Sep 17 00:00:00 2001 > From: Richard W.M. Jones<rjones at redhat.com> > Date: Mon, 7 Mar 2011 19:28:30 +0000 > Subject: [PATCH] generator: Introduce error code (errcode) concept. > > There was a lot of repeated code to map return types (eg. RErr) > to error cases (eg. -1 or NULL). > > This commit introduces an error code type and two functions to > map return types to error codes and error codes to strings. > --- > generator/generator_c.ml | 122 ++++++++++++++++++++++---------------- > generator/generator_capitests.ml | 56 ++++++++++-------- > generator/generator_daemon.ml | 40 +++++++------ > generator/generator_java.ml | 85 +++++++++++++++------------ > generator/generator_ocaml.ml | 58 ++++++++++-------- > generator/generator_php.ml | 57 ++++++++++-------- > generator/generator_python.ml | 46 ++++++++------ > generator/generator_ruby.ml | 40 +++++++----- > generator/generator_types.ml | 4 +- > generator/generator_utils.ml | 17 +++++- > generator/generator_utils.mli | 13 ++++- > 11 files changed, 315 insertions(+), 223 deletions(-)Adds more than it removes: that's a little disappointing. Still, I think it is worth it to have a single 'repository' of error codes. Incidentally, what's generator_utils.mli doing in there? Surely that's not under version control. Matt -- Matthew Booth, RHCA, RHCSS Red Hat Engineering, Virtualisation Team GPG ID: D33C3490 GPG FPR: 3733 612D 2D05 5458 8A8A 1600 3441 EA19 D33C 3490
Possibly Parallel Threads
- Testing github pull requests
- [PATCH 1/2] generator: Rename java_structs to camel_structs to better reflect their purpose
- [PATCH 0/7] Add tar compress, numericowner, excludes flags.
- [PATCH v2] Add tune2fs command.
- [PATCH 0/6] Allow non-optargs functions to gain optional arguments.