Pino Toscano
2015-Oct-21 10:11 UTC
[Libguestfs] [PATCH 1/2] generator: add a FilenameList parameter type
Mostly like StringList (so it can used in current StringList parameters), but checking client- and daemon-side that the elements are file names. --- generator/bindtests.ml | 3 ++- generator/c.ml | 29 ++++++++++++++++++++++++----- generator/csharp.ml | 4 ++-- generator/daemon.ml | 20 ++++++++++++++++++-- generator/erlang.ml | 2 +- generator/fish.ml | 10 ++++++---- generator/gobject.ml | 7 ++++--- generator/golang.ml | 9 ++++++--- generator/haskell.ml | 9 +++++---- generator/java.ml | 10 +++++----- generator/lua.ml | 6 +++--- generator/ocaml.ml | 7 ++++--- generator/perl.ml | 9 ++++++--- generator/php.ml | 13 ++++++++----- generator/python.ml | 13 +++++++------ generator/ruby.ml | 4 ++-- generator/tests_c_api.ml | 7 +++++-- generator/types.ml | 5 +++++ generator/utils.ml | 2 +- generator/xdr.ml | 3 ++- 20 files changed, 116 insertions(+), 56 deletions(-) diff --git a/generator/bindtests.ml b/generator/bindtests.ml index 0959704..020d059 100644 --- a/generator/bindtests.ml +++ b/generator/bindtests.ml @@ -163,7 +163,8 @@ fill_lvm_pv (guestfs_h *g, struct guestfs_lvm_pv *pv, size_t i) pr " fprintf (fp, \"\\n\");\n"; pr " }\n"; | OptString n -> pr " fprintf (fp, \"%%s\\n\", %s ? %s : \"null\");\n" n n - | StringList n | DeviceList n -> pr " print_strings (g, %s);\n" n + | StringList n | DeviceList n | FilenameList n -> + pr " print_strings (g, %s);\n" n | Bool n -> pr " fprintf (fp, \"%%s\\n\", %s ? \"true\" : \"false\");\n" n | Int n -> pr " fprintf (fp, \"%%d\\n\", %s);\n" n | Int64 n -> pr " fprintf (fp, \"%%\" PRIi64 \"\\n\", %s);\n" n diff --git a/generator/c.ml b/generator/c.ml index 055b683..f05da03 100644 --- a/generator/c.ml +++ b/generator/c.ml @@ -131,7 +131,7 @@ let rec generate_prototype ?(extern = true) ?(static = false) pr "const mountable_t *%s" n else pr "const char *%s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> next (); pr "char *const *%s" n | Bool n -> next (); pr "int %s" n @@ -1252,7 +1252,8 @@ and generate_client_actions hash () | DeviceList n | Key n | Pointer (_, n) - | GUID n -> + | GUID n + | FilenameList n -> pr " if (%s == NULL) {\n" n; pr " error (g, \"%%s: %%s: parameter cannot be NULL\",\n"; pr " \"%s\", \"%s\");\n" c_name n; @@ -1355,6 +1356,23 @@ and generate_client_actions hash () pr " }\n"; pr_newline := true + | FilenameList n -> + pr " {\n"; + pr " size_t i;\n"; + pr " for (i = 0; %s[i] != NULL; ++i) {\n" n; + pr " if (strchr (%s[i], '/') != NULL) {\n" n; + pr " error (g, \"%%s: %%s: '%%s' is not a file name\",\n"; + pr " \"%s\", \"%s\", %s[i]);\n" c_name n n; + 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"; + pr " }\n"; + pr_newline := true + (* not applicable *) | String _ | Device _ @@ -1383,7 +1401,7 @@ and generate_client_actions hash () let needs_i List.exists (function - | StringList _ | DeviceList _ -> true + | StringList _ | DeviceList _ | FilenameList _ -> true | _ -> false) args || List.exists (function | OStringList _ -> true @@ -1419,7 +1437,8 @@ and generate_client_actions hash () pr " else\n"; pr " fprintf (trace_buffer.fp, \" null\");\n" | StringList n - | DeviceList n -> (* string list *) + | DeviceList n + | FilenameList n -> (* string list *) pr " fputc (' ', trace_buffer.fp);\n"; pr " fputc ('\"', trace_buffer.fp);\n"; pr " for (i = 0; %s[i]; ++i) {\n" n; @@ -1735,7 +1754,7 @@ and generate_client_actions hash () 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 -> + | StringList n | DeviceList n | FilenameList 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 -> diff --git a/generator/csharp.ml b/generator/csharp.ml index e2bd25b..6022af1 100644 --- a/generator/csharp.ml +++ b/generator/csharp.ml @@ -195,7 +195,7 @@ namespace Guestfs | BufferIn n | GUID n -> pr ", [In] string %s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr ", [In] string[] %s" n | Bool n -> pr ", bool %s" n @@ -224,7 +224,7 @@ namespace Guestfs | BufferIn n | GUID n -> next (); pr "string %s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> next (); pr "string[] %s" n | Bool n -> next (); pr "bool %s" n diff --git a/generator/daemon.ml b/generator/daemon.ml index 1825de4..a950c17 100644 --- a/generator/daemon.ml +++ b/generator/daemon.ml @@ -255,7 +255,7 @@ cleanup_free_mountable (mountable_t *mountable) | Mountable n | Mountable_or_Path n -> pr " CLEANUP_FREE_MOUNTABLE mountable_t %s\n" n; pr " = { .device = NULL, .volume = NULL };\n" - | StringList n -> + | StringList n | FilenameList n -> pr " char **%s;\n" n | DeviceList n -> pr " CLEANUP_FREE_STRING_LIST char **%s = NULL;\n" n @@ -344,7 +344,23 @@ cleanup_free_mountable (mountable_t *mountable) n n (if is_filein then "cancel_receive ()" else ""); | String n | Key n | GUID n -> pr_args n | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n - | StringList n -> + | StringList n | FilenameList n as arg -> + (match arg with + | FilenameList n -> + pr " {\n"; + pr " size_t i;\n"; + pr " for (i = 0; i < args.%s.%s_len; ++i) {\n" n n; + pr " if (strchr (args.%s.%s_val[i], '/') != NULL) {\n" n n; + if is_filein then + pr " cancel_receive ();\n"; + pr " reply_with_error (\"%%s: '%%s' is not a file name\", __func__, args.%s.%s_val[i]);\n" + n n; + pr " goto done;\n"; + pr " }\n"; + pr " }\n"; + pr " }\n" + | _ -> () + ); pr " /* Ugly, but safe and avoids copying the strings. */\n"; pr " %s = realloc (args.%s.%s_val,\n" n n n; pr " sizeof (char *) * (args.%s.%s_len+1));\n" n n; diff --git a/generator/erlang.ml b/generator/erlang.ml index 5d4faaf..d5d30b4 100644 --- a/generator/erlang.ml +++ b/generator/erlang.ml @@ -310,7 +310,7 @@ extern int64_t get_int64 (ETERM *term); pr " ETERM *%s_bin = erl_iolist_to_binary (ARG (%d));\n" n i; pr " const void *%s = ERL_BIN_PTR (%s_bin);\n" n n; pr " size_t %s_size = ERL_BIN_SIZE (%s_bin);\n" n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " CLEANUP_FREE_STRING_LIST char **%s = get_string_list (ARG (%d));\n" n i | Bool n -> pr " int %s = get_bool (ARG (%d));\n" n i diff --git a/generator/fish.ml b/generator/fish.ml index 6f68e65..40b97d3 100644 --- a/generator/fish.ml +++ b/generator/fish.ml @@ -376,7 +376,8 @@ Guestfish will prompt for these separately." | BufferIn n -> pr " const char *%s;\n" n; pr " size_t %s_size;\n" n - | StringList n | DeviceList n -> pr " char **%s;\n" n + | StringList n | DeviceList n | FilenameList 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 @@ -464,7 +465,7 @@ Guestfish will prompt for these separately." | FileOut name -> pr " %s = file_out (argv[i++]);\n" name; pr " if (%s == NULL) goto out_%s;\n" name name - | StringList name | DeviceList name -> + | StringList name | DeviceList name | FilenameList name -> pr " %s = parse_string_list (argv[i++]);\n" name; pr " if (%s == NULL) goto out_%s;\n" name name | Key name -> @@ -678,7 +679,7 @@ Guestfish will prompt for these separately." | FileIn name -> pr " free_file_in (%s);\n" name; pr " out_%s:\n" name - | StringList name | DeviceList name -> + | StringList name | DeviceList name | FilenameList name -> pr " guestfs_int_free_string_list (%s);\n" name; pr " out_%s:\n" name | Pointer _ -> assert false @@ -917,7 +918,8 @@ and generate_fish_actions_pod () | GUID n -> pr " %s" n | OptString n -> pr " %s" n - | StringList n | DeviceList n -> pr " '%s ...'" n + | StringList n | DeviceList n | FilenameList n -> + pr " '%s ...'" n | Bool _ -> pr " true|false" | Int n -> pr " %s" n | Int64 n -> pr " %s" n diff --git a/generator/gobject.ml b/generator/gobject.ml index 8d5ac06..2c8b64e 100644 --- a/generator/gobject.ml +++ b/generator/gobject.ml @@ -85,7 +85,8 @@ let generate_gobject_proto name ?(single_line = true) | GUID n -> pr "const gchar *%s" n | StringList n - | DeviceList n -> + | DeviceList n + | FilenameList n -> pr "gchar *const *%s" n | BufferIn n -> pr "const guint8 *%s, gsize %s_size" n n @@ -1052,7 +1053,7 @@ guestfs_session_close (GuestfsSession *session, GError **err) pr " (transfer none) (type filename):" | StringList _ -> pr " (transfer none) (array zero-terminated=1) (element-type utf8): an array of strings" - | DeviceList _ -> + | DeviceList _ | FilenameList _ -> pr " (transfer none) (array zero-terminated=1) (element-type filename): an array of strings" | BufferIn n -> pr " (transfer none) (array length=%s_size) (element-type guint8): an array of binary data\n" n; @@ -1211,7 +1212,7 @@ guestfs_session_close (GuestfsSession *session, GError **err) | Pathname n | Dev_or_Path n | Mountable_or_Path n | OptString n | StringList n | DeviceList n | Key n | FileIn n | FileOut n - | GUID n -> + | GUID n | FilenameList n -> pr "%s" n | Pointer (_, n) -> pr "%s" n diff --git a/generator/golang.ml b/generator/golang.ml index b4b2482..faaf19e 100644 --- a/generator/golang.ml +++ b/generator/golang.ml @@ -315,7 +315,8 @@ func return_hashtable (argv **C.char) map[string]string { | GUID n -> pr "%s string" n | OptString n -> pr "%s *string" n | StringList n - | DeviceList n -> pr "%s []string" n + | DeviceList n + | FilenameList n -> pr "%s []string" n | BufferIn n -> pr "%s []byte" n | Pointer (_, n) -> pr "%s int64" n ) args; @@ -380,7 +381,8 @@ func return_hashtable (argv **C.char) map[string]string { pr " defer C.free (unsafe.Pointer (c_%s))\n" n; pr " }\n" | StringList n - | DeviceList n -> + | DeviceList n + | FilenameList n -> pr "\n"; pr " c_%s := arg_string_list (%s)\n" n n; pr " defer free_string_list (c_%s)\n" n @@ -455,7 +457,8 @@ func return_hashtable (argv **C.char) map[string]string { | FileIn n | FileOut n | GUID n -> pr "c_%s" n | StringList n - | DeviceList n -> pr "c_%s" n + | DeviceList n + | FilenameList n -> pr "c_%s" n | BufferIn n -> pr "c_%s, C.size_t (len (%s))" n n | Pointer _ -> pr "nil" ) args; diff --git a/generator/haskell.ml b/generator/haskell.ml index 96682e6..59c5859 100644 --- a/generator/haskell.ml +++ b/generator/haskell.ml @@ -147,7 +147,8 @@ assocListOfHashtable (a:b:rest) = (a,b) : assocListOfHashtable rest | BufferIn n -> pr "withCStringLen %s $ \\(%s, %s_size) -> " n n n | OptString n -> pr "maybeWith withCString %s $ \\%s -> " n n - | StringList n | DeviceList n -> pr "withMany withCString %s $ \\%s -> withArray0 nullPtr %s $ \\%s -> " n n n n + | StringList n | DeviceList n | FilenameList n -> + pr "withMany withCString %s $ \\%s -> withArray0 nullPtr %s $ \\%s -> " n n n n | Bool _ | Int _ | Int64 _ | Pointer _ -> () ) args; (* Convert integer arguments. *) @@ -161,7 +162,7 @@ assocListOfHashtable (a:b:rest) = (a,b) : assocListOfHashtable rest | Pathname n | Device n | Mountable n | Dev_or_Path n | Mountable_or_Path n | String n | OptString n - | StringList n | DeviceList n + | StringList n | DeviceList n | FilenameList n | Key n | GUID n -> n | BufferIn n -> sprintf "%s (fromIntegral %s_size)" n n ) args in @@ -224,7 +225,7 @@ and generate_haskell_prototype ~handle ?(hs = false) (ret, args, optargs) pr "CString -> CInt" | OptString _ -> pr "CString" - | StringList _ | DeviceList _ -> + | StringList _ | DeviceList _ | FilenameList _ -> pr "Ptr CString" | Bool _ -> pr "CInt" | Int _ -> pr "CInt" @@ -267,7 +268,7 @@ and generate_haskell_prototype ~handle ?(hs = false) (ret, args, optargs) pr "String" | OptString _ -> pr "Maybe String" - | StringList _ | DeviceList _ -> + | StringList _ | DeviceList _ | FilenameList _ -> pr "[String]" | Bool _ -> pr "Bool" | Int _ -> pr "Int" diff --git a/generator/java.ml b/generator/java.ml index 4c89197..93266cb 100644 --- a/generator/java.ml +++ b/generator/java.ml @@ -485,7 +485,7 @@ and generate_java_prototype ?(public=false) ?(privat=false) ?(native=false) pr "String %s" n | BufferIn n -> pr "byte[] %s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr "String[] %s" n | Bool n -> pr "boolean %s" n @@ -846,7 +846,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) pr ", jstring j%s" n | BufferIn n -> pr ", jbyteArray j%s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr ", jobjectArray j%s" n | Bool n -> pr ", jboolean j%s" n @@ -917,7 +917,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) | BufferIn n -> pr " char *%s;\n" n; pr " size_t %s_size;\n" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " size_t %s_len;\n" n; pr " char **%s;\n" n | Bool n @@ -979,7 +979,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) | BufferIn n -> pr " %s = (char *) (*env)->GetByteArrayElements (env, j%s, NULL);\n" n n; pr " %s_size = (*env)->GetArrayLength (env, j%s);\n" n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " %s_len = (*env)->GetArrayLength (env, j%s);\n" n n; pr " %s = guestfs_int_safe_malloc (g, sizeof (char *) * (%s_len+1));\n" n n; pr " for (i = 0; i < %s_len; ++i) {\n" n; @@ -1044,7 +1044,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) pr " (*env)->ReleaseStringUTFChars (env, j%s, %s);\n" n n | BufferIn n -> pr " (*env)->ReleaseByteArrayElements (env, j%s, (jbyte *) %s, 0);\n" n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " for (i = 0; i < %s_len; ++i) {\n" n; pr " jobject o = (*env)->GetObjectArrayElement (env, j%s, i);\n" n; diff --git a/generator/lua.ml b/generator/lua.ml index 630d805..7c0e93f 100644 --- a/generator/lua.ml +++ b/generator/lua.ml @@ -474,7 +474,7 @@ guestfs_lua_delete_event_callback (lua_State *L) pr " size_t %s_size;\n" n; | OptString n -> pr " const char *%s;\n" n; - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " char **%s;\n" n | Bool n -> pr " int %s;\n" n | Int n -> pr " int %s;\n" n @@ -504,7 +504,7 @@ guestfs_lua_delete_event_callback (lua_State *L) pr " %s = luaL_checklstring (L, %d, &%s_size);\n" n i n | OptString n -> pr " %s = luaL_optstring (L, %d, NULL);\n" n i - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " %s = get_string_list (L, %d);\n" n i | Bool n -> pr " %s = lua_toboolean (L, %d);\n" n i @@ -564,7 +564,7 @@ guestfs_lua_delete_event_callback (lua_State *L) | BufferIn _ | OptString _ | Bool _ | Int _ | Int64 _ | Pointer _ | GUID _ -> () - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " free (%s);\n" n ) args; List.iter ( diff --git a/generator/ocaml.ml b/generator/ocaml.ml index 5d92fcb..4bbe5bc 100644 --- a/generator/ocaml.ml +++ b/generator/ocaml.ml @@ -600,7 +600,7 @@ copy_table (char * const * argv) | BufferIn n -> pr " size_t %s_size = caml_string_length (%sv);\n" n n; pr " char *%s = guestfs_int_safe_memdup (g, String_val (%sv), %s_size);\n" n n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " char **%s = ocaml_guestfs_strings_val (g, %sv);\n" n n | Bool n -> pr " int %s = Bool_val (%sv);\n" n n @@ -677,7 +677,7 @@ copy_table (char * const * argv) | OptString n | FileIn n | FileOut n | BufferIn n | Key n | GUID n -> pr " free (%s);\n" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " guestfs_int_free_string_list (%s);\n" n; | Bool _ | Int _ | Int64 _ | Pointer _ -> () ) args; @@ -850,7 +850,8 @@ and generate_ocaml_function_type ?(extra_unit = false) (ret, args, optargs) | FileIn _ | FileOut _ | BufferIn _ | Key _ | GUID _ -> pr "string -> " | OptString _ -> pr "string option -> " - | StringList _ | DeviceList _ -> pr "string array -> " + | StringList _ | DeviceList _ | FilenameList _ -> + pr "string array -> " | Bool _ -> pr "bool -> " | Int _ -> pr "int -> " | Int64 _ | Pointer _ -> pr "int64 -> " diff --git a/generator/perl.ml b/generator/perl.ml index dcd180f..81973e4 100644 --- a/generator/perl.ml +++ b/generator/perl.ml @@ -365,7 +365,8 @@ PREINIT: * to add 1 to the ST(x) operator. *) pr " char *%s = SvOK(ST(%d)) ? SvPV_nolen(ST(%d)) : NULL;\n" n (i+1) (i+1) - | StringList n | DeviceList n -> pr " char **%s;\n" n + | StringList n | DeviceList n | FilenameList 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 @@ -510,7 +511,8 @@ PREINIT: | OptString _ | Bool _ | Int _ | Int64 _ | FileIn _ | FileOut _ | BufferIn _ | Key _ | Pointer _ | GUID _ -> () - | StringList n | DeviceList n -> pr " free (%s);\n" n + | StringList n | DeviceList n | FilenameList n -> + pr " free (%s);\n" n ) args; (* Check return value for errors and return it if necessary. *) @@ -955,6 +957,7 @@ errnos: | Int n -> pr "[ '%s', 'int', %d ]" n i | Int64 n -> pr "[ '%s', 'int64', %d ]" n i | Pointer (t, n) -> pr "[ '%s', 'pointer(%s)', %d ]" n t i + | FilenameList n -> pr "[ '%s', 'string(path) list', %d ]" n i in pr " args => [\n"; iteri (fun i arg -> @@ -1113,7 +1116,7 @@ and generate_perl_prototype name (ret, args, optargs) | OptString n | Bool n | Int n | Int64 n | FileIn n | FileOut n | BufferIn n | Key n | Pointer (_, n) | GUID n -> pr "$%s" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr "\\@%s" n ) args; List.iter ( diff --git a/generator/php.ml b/generator/php.ml index b49bf60..cb0543a 100644 --- a/generator/php.ml +++ b/generator/php.ml @@ -240,7 +240,8 @@ PHP_FUNCTION (guestfs_last_error) pr " char *%s = NULL;\n" n; pr " int %s_size;\n" n | StringList n - | DeviceList n -> + | DeviceList n + | FilenameList n -> pr " zval *z_%s;\n" n; pr " char **%s;\n" n; | Bool n -> @@ -284,7 +285,7 @@ PHP_FUNCTION (guestfs_last_error) | FileIn n | FileOut n | BufferIn n | Key n | GUID n -> "s" | OptString n -> "s!" - | StringList n | DeviceList n -> "a" + | StringList n | DeviceList n | FilenameList n -> "a" | Bool n -> "b" | Int n | Int64 n -> "l" | Pointer _ -> "" @@ -319,7 +320,7 @@ PHP_FUNCTION (guestfs_last_error) | FileIn n | FileOut n | BufferIn n | Key n | OptString n | GUID n -> pr ", &%s, &%s_size" n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr ", &z_%s" n | Bool n -> pr ", &%s" n @@ -372,7 +373,8 @@ PHP_FUNCTION (guestfs_last_error) pr "\n" | BufferIn n -> () | StringList n - | DeviceList n -> + | DeviceList n + | FilenameList n -> pr " %s = get_stringlist (z_%s);\n" n n; pr "\n" | Bool _ | Int _ | Int64 _ -> () @@ -453,7 +455,8 @@ PHP_FUNCTION (guestfs_last_error) | OptString n | GUID n -> () | BufferIn n -> () | StringList n - | DeviceList n -> + | DeviceList n + | FilenameList n -> pr " guestfs_efree_stringlist (%s);\n" n; pr "\n" | Bool _ | Int _ | Int64 _ | Pointer _ -> () diff --git a/generator/python.ml b/generator/python.ml index 1e043fc..9b3037c 100644 --- a/generator/python.ml +++ b/generator/python.ml @@ -292,7 +292,7 @@ put_table (char * const * const argv) | BufferIn n -> pr " const char *%s;\n" n; pr " Py_ssize_t %s_size;\n" n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " PyObject *py_%s;\n" n; pr " char **%s = NULL;\n" n | Bool n -> pr " int %s;\n" n @@ -326,7 +326,7 @@ put_table (char * const * const argv) | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | GUID _ -> pr "s" | OptString _ -> pr "z" - | StringList _ | DeviceList _ -> pr "O" + | StringList _ | DeviceList _ | FilenameList _ -> pr "O" | Bool _ -> pr "i" (* XXX Python has booleans? *) | Int _ -> pr "i" | Int64 _ -> @@ -349,7 +349,8 @@ put_table (char * const * const argv) | Dev_or_Path n | Mountable_or_Path n | String n | Key n | FileIn n | FileOut n | GUID n -> pr ", &%s" n | OptString n -> pr ", &%s" n - | StringList n | DeviceList n -> pr ", &py_%s" n + | StringList n | DeviceList n | FilenameList n -> + pr ", &py_%s" n | Bool n -> pr ", &%s" n | Int n -> pr ", &%s" n | Int64 n -> pr ", &%s" n @@ -372,7 +373,7 @@ put_table (char * const * const argv) | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | GUID _ -> () - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " %s = get_string_list (py_%s);\n" n n; pr " if (!%s) goto out;\n" n | Pointer (_, n) -> @@ -522,7 +523,7 @@ put_table (char * const * const argv) | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | Pointer _ | GUID _ -> () - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " free (%s);\n" n ) args; @@ -829,7 +830,7 @@ class GuestFS(object): | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | GUID _ -> () - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " %s = list (%s)\n" n n | Pointer (_, n) -> pr " %s = %s.c_pointer()\n" n n diff --git a/generator/ruby.ml b/generator/ruby.ml index cb187b0..e2761c7 100644 --- a/generator/ruby.ml +++ b/generator/ruby.ml @@ -582,7 +582,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) pr " size_t %s_size = RSTRING_LEN (%sv);\n" n n | OptString n -> pr " const char *%s = !NIL_P (%sv) ? StringValueCStr (%sv) : NULL;\n" n n n - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " char **%s;\n" n; pr " Check_Type (%sv, T_ARRAY);\n" n; pr " {\n"; @@ -677,7 +677,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) | Dev_or_Path _ | Mountable_or_Path _ | String _ | Key _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ | Int64 _ | BufferIn _ | Pointer _ | GUID _ -> () - | StringList n | DeviceList n -> + | StringList n | DeviceList n | FilenameList n -> pr " free (%s);\n" n ) args; diff --git a/generator/tests_c_api.ml b/generator/tests_c_api.ml index 1522eff..6be753f 100644 --- a/generator/tests_c_api.ml +++ b/generator/tests_c_api.ml @@ -419,7 +419,8 @@ and generate_test_command_call ?(expect_error = false) ?test ?ret test_name cmd | DeviceList _, "", sym -> pr " const char *const %s[1] = { NULL };\n" sym | StringList _, arg, sym - | DeviceList _, arg, sym -> + | DeviceList _, arg, sym + | FilenameList _, arg, sym -> let strs = string_split " " arg in iteri ( fun i str -> @@ -527,7 +528,9 @@ and generate_test_command_call ?(expect_error = false) ?test ?ret test_name cmd | GUID _, _, sym -> pr ", %s" sym | BufferIn _, _, sym -> pr ", %s, %s_size" sym sym | FileOut _, arg, _ -> pr ", \"%s\"" (c_quote arg) - | StringList _, _, sym | DeviceList _, _, sym -> pr ", (char **) %s" sym + | StringList _, _, sym | DeviceList _, _, sym + | FilenameList _, _, sym -> + pr ", (char **) %s" sym | Int _, arg, _ -> let i try int_of_string arg diff --git a/generator/types.ml b/generator/types.ml index 83a6a98..0d22e0c 100644 --- a/generator/types.ml +++ b/generator/types.ml @@ -190,6 +190,11 @@ and argt * guestfs_int_validate_guid. *) | GUID of string + (* List of file names only, where the list cannot be NULL, + * and each element cannot be NULL, empty, or anything different than + * a simple file name (i.e. neither absolute nor relative paths). + *) + | FilenameList of string and optargs = optargt list diff --git a/generator/utils.ml b/generator/utils.ml index 1b00ce5..7d47430 100644 --- a/generator/utils.ml +++ b/generator/utils.ml @@ -254,7 +254,7 @@ let name_of_argt = function | Mountable_or_Path n | String n | OptString n | StringList n | DeviceList n | Bool n | Int n | Int64 n | FileIn n | FileOut n | BufferIn n | Key n | Pointer (_, n) - | GUID n -> n + | GUID n | FilenameList n -> n let name_of_optargt = function | OBool n | OInt n | OInt64 n | OString n | OStringList n -> n diff --git a/generator/xdr.ml b/generator/xdr.ml index 37a52b7..037be1e 100644 --- a/generator/xdr.ml +++ b/generator/xdr.ml @@ -115,7 +115,8 @@ let generate_xdr () | Key n | GUID n -> pr " string %s<>;\n" n | OptString n -> pr " guestfs_str *%s;\n" n - | StringList n | DeviceList n -> pr " guestfs_str %s<>;\n" n + | StringList n | DeviceList n | FilenameList n -> + pr " guestfs_str %s<>;\n" n | Bool n -> pr " bool %s;\n" n | Int n -> pr " int %s;\n" n | Int64 n -> pr " int64_t %s;\n" n -- 2.1.0
Pino Toscano
2015-Oct-21 10:11 UTC
[Libguestfs] [PATCH 2/2] actions: turn some params into FilenameList (RHBZ#1174551).
Use FilenameList as type for lists of file names, as used in some listing-alike APIs. This way we can ensure anything different than just file names in those lists is rejected outright. As a consequence, test-big-dirs.pl does not need to prepend the directory name anymore before calling listing-alike APIs: previously they didn't fail, but the returned lists contained only invalid elements (and only their size was checked). Furthermore, add a new regression test for it. --- generator/actions.ml | 14 +++++----- tests/bigdirs/test-big-dirs.pl | 1 - tests/regressions/Makefile.am | 2 ++ tests/regressions/rhbz1174551.sh | 58 ++++++++++++++++++++++++++++++++++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) create mode 100755 tests/regressions/rhbz1174551.sh diff --git a/generator/actions.ml b/generator/actions.ml index 10acb7c..62176ab 100644 --- a/generator/actions.ml +++ b/generator/actions.ml @@ -2655,7 +2655,7 @@ See also C<guestfs_write>." }; { defaults with name = "lstatlist"; added = (1, 0, 77); - style = RStructList ("statbufs", "stat"), [Pathname "path"; StringList "names"], []; + style = RStructList ("statbufs", "stat"), [Pathname "path"; FilenameList "names"], []; deprecated_by = Some "lstatnslist"; shortdesc = "lstat on multiple files"; longdesc = "\ @@ -2675,7 +2675,7 @@ for getting extended attributes." }; { defaults with name = "lstatnslist"; added = (1, 27, 53); - style = RStructList ("statbufs", "statns"), [Pathname "path"; StringList "names"], []; + style = RStructList ("statbufs", "statns"), [Pathname "path"; FilenameList "names"], []; shortdesc = "lstat on multiple files"; longdesc = "\ This call allows you to perform the C<guestfs_lstatns> operation @@ -2694,7 +2694,7 @@ for getting extended attributes." }; { defaults with name = "lxattrlist"; added = (1, 0, 77); - style = RStructList ("xattrs", "xattr"), [Pathname "path"; StringList "names"], []; + style = RStructList ("xattrs", "xattr"), [Pathname "path"; FilenameList "names"], []; optional = Some "linuxxattrs"; shortdesc = "lgetxattr on multiple files"; longdesc = "\ @@ -2719,7 +2719,7 @@ for getting standard stats." }; { defaults with name = "readlinklist"; added = (1, 0, 77); - style = RStringList "links", [Pathname "path"; StringList "names"], []; + style = RStringList "links", [Pathname "path"; FilenameList "names"], []; shortdesc = "readlink on multiple files"; longdesc = "\ This call allows you to do a C<readlink> operation @@ -7703,7 +7703,7 @@ yourself (Augeas support makes this relatively easy)." }; { defaults with name = "internal_lxattrlist"; added = (1, 19, 32); - style = RStructList ("xattrs", "xattr"), [Pathname "path"; StringList "names"], []; + style = RStructList ("xattrs", "xattr"), [Pathname "path"; FilenameList "names"], []; proc_nr = Some 205; visibility = VInternal; optional = Some "linuxxattrs"; @@ -7733,7 +7733,7 @@ into smaller groups of names." }; { defaults with name = "internal_readlinklist"; added = (1, 19, 32); - style = RStringList "links", [Pathname "path"; StringList "names"], []; + style = RStringList "links", [Pathname "path"; FilenameList "names"], []; proc_nr = Some 206; visibility = VInternal; shortdesc = "readlink on multiple files"; @@ -12220,7 +12220,7 @@ This is the same as the L<lstat(2)> system call." }; { defaults with name = "internal_lstatnslist"; added = (1, 27, 53); - style = RStructList ("statbufs", "statns"), [Pathname "path"; StringList "names"], []; + style = RStructList ("statbufs", "statns"), [Pathname "path"; FilenameList "names"], []; proc_nr = Some 423; visibility = VInternal; shortdesc = "lstat on multiple files"; diff --git a/tests/bigdirs/test-big-dirs.pl b/tests/bigdirs/test-big-dirs.pl index 40038b7..5bcc71c 100755 --- a/tests/bigdirs/test-big-dirs.pl +++ b/tests/bigdirs/test-big-dirs.pl @@ -63,7 +63,6 @@ for (my $i = 0; $i < $nr_files; ++$i) { # Check that lstatlist, lxattrlist and readlinklist return the # expected number of entries. my @a; -@filenames = map { "/dir/$_" } @filenames; @a = $g->lstatlist ("/dir", \@filenames); die unless @a == $nr_files; diff --git a/tests/regressions/Makefile.am b/tests/regressions/Makefile.am index 026987b..18a4f37 100644 --- a/tests/regressions/Makefile.am +++ b/tests/regressions/Makefile.am @@ -43,6 +43,7 @@ EXTRA_DIST = \ rhbz1044014.xml \ rhbz1054761.sh \ rhbz1091803.sh \ + rhbz1174551.sh \ rhbz1175196.sh \ rhbz1232192.sh \ rhbz1232192.xml \ @@ -71,6 +72,7 @@ TESTS = \ rhbz1055452 \ rhbz1091803.sh \ rhbz1011907-1165785.sh \ + rhbz1174551.sh \ rhbz1175196.sh \ rhbz1232192.sh \ test-big-heap \ diff --git a/tests/regressions/rhbz1174551.sh b/tests/regressions/rhbz1174551.sh new file mode 100755 index 0000000..fdc595c --- /dev/null +++ b/tests/regressions/rhbz1174551.sh @@ -0,0 +1,58 @@ +#!/bin/bash - +# libguestfs +# Copyright (C) 2015 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 +# the Free Software Foundation; either version 2 of the License, or +# (at your option) any later version. +# +# This program is distributed in the hope that it will be useful, +# but WITHOUT ANY WARRANTY; without even the implied warranty of +# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +# GNU General Public License for more details. +# +# You should have received a copy of the GNU General Public License +# along with this program; if not, write to the Free Software +# Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA. + +# Regression test for: +# https://bugzilla.redhat.com/show_bug.cgi?id=1174551 +# check that list-alike APIs accept only relative paths and reject +# absolute ones + +set -e +export LANG=C + +if [ -n "$SKIP_TEST_RHBZ1174551_SH" ]; then + echo "$0: test skipped because environment variable is set." + exit 77 +fi + +if [ ! -s ../guests/fedora.img ]; then + echo "$0: test skipped because there is no fedora.img" + exit 77 +fi + +rm -f test.error + +$VG guestfish --ro -a ../guests/fedora.img -i <<EOF 2>test.error +# valid invocations +lstatlist /etc "fedora-release sysconfig" +lstatnslist /etc "fedora-release sysconfig" +readlinklist /bin "test5" + +# invalid invocations +-lstatlist / "/bin" +-lstatnslist / "/bin" +-lstatlist /etc "../bin sysconfig/network" +-readlinklist /etc "/bin/test5" +EOF + +# check the number of errors in the log +if [ $(cat test.error | wc -l) -ne 4 ]; then + echo "$0: " + exit 1 +fi + +rm test.error -- 2.1.0
Richard W.M. Jones
2015-Oct-21 10:55 UTC
Re: [Libguestfs] [PATCH 1/2] generator: add a FilenameList parameter type
ACK series. Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://libguestfs.org
Possibly Parallel Threads
- [PATCH 1/2] generator: Simplify the handling of string parameters.
- [PATCH 1/2] generator: add a RelativePathnameList parameter type
- [PATCH 1/4] php: fix invalid memory access with OptString
- [PATCH 01/12] generator: Add new Mountable argument type
- [PATCH 2/2] generator: Don't permit certain String/stringt combinations.