From: Jim Meyering <meyering(a)redhat.com&gt; --- daemon/augeas.c | 2 +- daemon/blockdev.c | 2 +- daemon/command.c | 12 +++--- daemon/daemon.h | 6 +- daemon/debug.c | 2 +- daemon/guestfsd.c | 20 +++++----- daemon/lvm.c | 12 ++---- daemon/sfdisk.c | 10 ++-- fish/fish.c | 6 +- fish/fish.h | 6 +- src/generator.ml | 107 +++++++++++++++++++++++++++++----------------------- 11 files changed, 97 insertions(+), 88 deletions(-) diff --git a/daemon/augeas.c b/daemon/augeas.c index 7de3624..3628219 100644 --- a/daemon/augeas.c +++ b/daemon/augeas.c @@ -376,7 +376,7 @@ do_aug_ls (const char *path) if (matches == NULL) return NULL; /* do_aug_match has already sent the error */ - sort_strings (matches, count_strings (matches)); + sort_strings (matches, count_strings ((void *) matches)); return matches; /* Caller frees. */ #else reply_with_error ("%s is not available", __func__); diff --git a/daemon/blockdev.c b/daemon/blockdev.c index 0745007..e0ac350 100644 --- a/daemon/blockdev.c +++ b/daemon/blockdev.c @@ -53,7 +53,7 @@ call_blockdev (const char *device, char *switc, int extraarg, int prints) } else argv[2] = device; - r = commandv (&out, &err, argv); + r = commandv (&out, &err, (char **) argv); if (r == -1) { reply_with_error ("%s: %s", argv[0], err); diff --git a/daemon/command.c b/daemon/command.c index 9e23e57..9fbbd95 100644 --- a/daemon/command.c +++ b/daemon/command.c @@ -27,7 +27,7 @@ #include "actions.h" char * -do_command (char **argv) +do_command (char *const *argv) { char *out, *err; int r; @@ -113,7 +113,7 @@ do_command (char **argv) } char ** -do_command_lines (char **argv) +do_command_lines (char *const *argv) { char *out; char **lines; @@ -134,15 +134,15 @@ do_command_lines (char **argv) char * do_sh (const char *command) { - char *argv[] = { "/bin/sh", "-c", command, NULL }; + const char *argv[] = { "/bin/sh", "-c", command, NULL }; - return do_command (argv); + return do_command ((char **) argv); } char ** do_sh_lines (const char *command) { - char *argv[] = { "/bin/sh", "-c", command, NULL }; + const char *argv[] = { "/bin/sh", "-c", command, NULL }; - return do_command_lines (argv); + return do_command_lines ((char **) argv); } diff --git a/daemon/daemon.h b/daemon/daemon.h index cbcab63..2a2b840 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -40,7 +40,7 @@ extern int xwrite (int sock, const void *buf, size_t len); extern int xread (int sock, void *buf, size_t len); extern int add_string (char ***argv, int *size, int *alloc, const char *str); -extern int count_strings (char * const* const argv); +extern int count_strings (char *const *argv); extern void sort_strings (char **argv, int len); extern void free_strings (char **argv); extern void free_stringslen (char **argv, int len); @@ -48,9 +48,9 @@ extern void free_stringslen (char **argv, int len); extern int command (char **stdoutput, char **stderror, const char *name, ...); extern int commandr (char **stdoutput, char **stderror, const char *name, ...); extern int commandv (char **stdoutput, char **stderror, - char * const* const argv); + char *const *argv); extern int commandrv (char **stdoutput, char **stderror, - char * const* const argv); + char const* const *argv); extern char **split_lines (char *str); diff --git a/daemon/debug.c b/daemon/debug.c index 58a5061..b428588 100644 --- a/daemon/debug.c +++ b/daemon/debug.c @@ -67,7 +67,7 @@ static struct cmd cmds[] = { #endif char * -do_debug (const char *subcmd MAYBE_UNUSED, char **argv MAYBE_UNUSED) +do_debug (const char *subcmd MAYBE_UNUSED, char *const *argv MAYBE_UNUSED) { #if ENABLE_DEBUG_COMMAND int argc, i; diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index b013661..fad49fb 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -351,7 +351,7 @@ add_string (char ***argv, int *size, int *alloc, const char *str) } int -count_strings (char * const* const argv) +count_strings (char *const *argv) { int argc; @@ -403,7 +403,7 @@ int command (char **stdoutput, char **stderror, const char *name, ...) { va_list args; - char **argv, **p; + const char **argv; char *s; int i, r; @@ -420,7 +420,7 @@ command (char **stdoutput, char **stderror, const char *name, ...) va_start (args, name); while ((s = va_arg (args, char *)) != NULL) { - p = realloc (argv, sizeof (char *) * (++i)); + const char **p = realloc (argv, sizeof (char *) * (++i)); if (p == NULL) { perror ("realloc"); free (argv); @@ -434,7 +434,7 @@ command (char **stdoutput, char **stderror, const char *name, ...) va_end (args); - r = commandv (stdoutput, stderror, argv); + r = commandv (stdoutput, stderror, (char **) argv); /* NB: Mustn't free the strings which are on the stack. */ free (argv); @@ -450,7 +450,7 @@ int commandr (char **stdoutput, char **stderror, const char *name, ...) { va_list args; - char **argv, **p; + const char **argv; char *s; int i, r; @@ -467,7 +467,7 @@ commandr (char **stdoutput, char **stderror, const char *name, ...) va_start (args, name); while ((s = va_arg (args, char *)) != NULL) { - p = realloc (argv, sizeof (char *) * (++i)); + const char **p = realloc (argv, sizeof (char *) * (++i)); if (p == NULL) { perror ("realloc"); free (argv); @@ -491,11 +491,11 @@ commandr (char **stdoutput, char **stderror, const char *name, ...) /* Same as 'command', but passing an argv. */ int -commandv (char **stdoutput, char **stderror, char * const* const argv) +commandv (char **stdoutput, char **stderror, char *const *argv) { int r; - r = commandrv (stdoutput, stderror, argv); + r = commandrv (stdoutput, stderror, (void *) argv); if (r == 0) return 0; else @@ -503,7 +503,7 @@ commandv (char **stdoutput, char **stderror, char * const* const argv) } int -commandrv (char **stdoutput, char **stderror, char * const* const argv) +commandrv (char **stdoutput, char **stderror, char const* const *argv) { int so_size = 0, se_size = 0; int so_fd[2], se_fd[2]; @@ -547,7 +547,7 @@ commandrv (char **stdoutput, char **stderror, char * const* const argv) close (so_fd[1]); close (se_fd[1]); - execvp (argv[0], argv); + execvp (argv[0], (void *) argv); perror (argv[0]); _exit (1); } diff --git a/daemon/lvm.c b/daemon/lvm.c index ddc125c..742b40b 100644 --- a/daemon/lvm.c +++ b/daemon/lvm.c @@ -192,16 +192,12 @@ do_pvcreate (const char *device) } int -do_vgcreate (const char *volgroup, char **physvols) +do_vgcreate (const char *volgroup, char *const *physvols) { char *err; int r, argc, i; const char **argv; - /* Check they are devices and also do device name translation. */ - for (i = 0; physvols[i] != NULL; ++i) - RESOLVE_DEVICE (physvols[i], return -1); - argc = count_strings (physvols) + 3; argv = malloc (sizeof (char *) * (argc + 1)); if (argv == NULL) { @@ -214,7 +210,7 @@ do_vgcreate (const char *volgroup, char **physvols) for (i = 3; i <= argc; ++i) argv[i] = physvols[i-3]; - r = commandv (NULL, &err, argv); + r = commandv (NULL, &err, (char **) argv); if (r == -1) { reply_with_error ("%s", err); free (err); @@ -424,7 +420,7 @@ do_pvresize (const char *device) } int -do_vg_activate (int activate, char **volgroups) +do_vg_activate (int activate, char *const *volgroups) { char *err; int r, i, argc; @@ -444,7 +440,7 @@ do_vg_activate (int activate, char **volgroups) for (i = 4; i <= argc; ++i) argv[i] = volgroups[i-4]; - r = commandv (NULL, &err, argv); + r = commandv (NULL, &err, (char **) argv); if (r == -1) { reply_with_error ("vgchange: %s", err); free (err); diff --git a/daemon/sfdisk.c b/daemon/sfdisk.c index 8a5a46b..6d49387 100644 --- a/daemon/sfdisk.c +++ b/daemon/sfdisk.c @@ -32,7 +32,7 @@ static int sfdisk (const char *device, int n, int cyls, int heads, int sectors, const char *extra_flag, - char * const* const lines) + char *const *lines) { FILE *fp; char buf[256]; @@ -96,7 +96,7 @@ sfdisk (const char *device, int n, int cyls, int heads, int sectors, int do_sfdisk (const char *device, int cyls, int heads, int sectors, - char **lines) + char *const *lines) { return sfdisk (device, 0, cyls, heads, sectors, NULL, lines); } @@ -105,13 +105,13 @@ int do_sfdisk_N (const char *device, int n, int cyls, int heads, int sectors, const char *line) { - const char *lines[2] = { line, NULL }; + char const *const lines[2] = { line, NULL }; - return sfdisk (device, n, cyls, heads, sectors, NULL, lines); + return sfdisk (device, n, cyls, heads, sectors, NULL, (void *) lines); } int -do_sfdiskM (const char *device, char **lines) +do_sfdiskM (const char *device, char *const *lines) { return sfdisk (device, 0, 0, 0, 0, "-uM", lines); } diff --git a/fish/fish.c b/fish/fish.c index f31f4ad..830617b 100644 --- a/fish/fish.c +++ b/fish/fish.c @@ -974,7 +974,7 @@ free_strings (char **argv) } int -count_strings (char * const * const argv) +count_strings (char *const *argv) { int c; @@ -984,7 +984,7 @@ count_strings (char * const * const argv) } void -print_strings (char * const * const argv) +print_strings (char *const *argv) { int argc; @@ -993,7 +993,7 @@ print_strings (char * const * const argv) } void -print_table (char * const * const argv) +print_table (char *const *argv) { int i; diff --git a/fish/fish.h b/fish/fish.h index 772d200..a6cc8c9 100644 --- a/fish/fish.h +++ b/fish/fish.h @@ -39,9 +39,9 @@ extern void pod2text (const char *name, const char *shortdesc, const char *body) extern void list_builtin_commands (void); extern void display_builtin_command (const char *cmd); extern void free_strings (char **argv); -extern int count_strings (char * const * const argv); -extern void print_strings (char * const * const argv); -extern void print_table (char * const * const argv); +extern int count_strings (char *const *argv); +extern void print_strings (char *const *argv); +extern void print_table (char *const *argv); extern int launch (guestfs_h *); extern int is_true (const char *str); extern char **parse_string_list (const char *str); diff --git a/src/generator.ml b/src/generator.ml index 85344aa..8a8d66e 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -139,6 +139,7 @@ and argt = | Dev_or_Path of string (* /dev device name or Pathname, cannot be NULL *) | OptString of string (* const char *name, may be NULL *) | StringList of string(* list of strings (each string cannot be NULL) *) + | DeviceList of string(* list of Device names (each cannot be NULL) *) | Bool of string (* boolean *) | Int of string (* int (smallish ints, signed, <= 31 bits) *) (* These are treated as filenames (simple string parameters) in @@ -1307,7 +1308,7 @@ This creates an LVM physical volume on the named C<device>, where C<device> should usually be a partition name such as C</dev/sda1>."); - ("vgcreate", (RErr, [String "volgroup"; StringList "physvols"]), 40, [], + ("vgcreate", (RErr, [String "volgroup"; DeviceList "physvols"]), 40, [], [InitEmpty, Always, TestOutputList ( [["sfdiskM"; "/dev/sda"; ",100 ,200 ,"]; ["pvcreate"; "/dev/sda1"]; @@ -3841,7 +3842,8 @@ let mapi f xs = loop 0 xs let name_of_argt = function - | Pathname n | Device n | Dev_or_Path n | String n | OptString n | StringList n | Bool n | Int n + | Pathname n | Device n | Dev_or_Path n | String n | OptString n + | StringList n | DeviceList n | Bool n | Int n | FileIn n | FileOut n -> n let java_name_of_struct typ = @@ -4230,7 +4232,7 @@ and generate_xdr () = function | Pathname n | Device n | Dev_or_Path n | String n -> pr " string %s<>;\n" n | OptString n -> pr " str *%s;\n" n - | StringList n -> pr " str %s<>;\n" n + | StringList n | DeviceList n -> pr " str %s<>;\n" n | Bool n -> pr " bool %s;\n" n | Int n -> pr " int %s;\n" n | FileIn _ | FileOut _ -> () @@ -4593,7 +4595,7 @@ check_state (guestfs_h *g, const char *caller) pr " args.%s = (char *) %s;\n" n n | OptString n -> pr " args.%s = %s ? (char **) &%s : NULL;\n" n n n - | StringList 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 -> @@ -4798,7 +4800,7 @@ and generate_daemon_actions () = | Pathname n | String n -> () | OptString n -> pr " char *%s;\n" n - | StringList 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 | FileIn _ | FileOut _ -> () @@ -4818,6 +4820,16 @@ and generate_daemon_actions () = 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; + 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 -> @@ -4832,14 +4844,14 @@ and generate_daemon_actions () = | String n -> pr_args n | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n | StringList 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; - pr " reply_with_perror (\"realloc\");\n"; - pr " goto done;\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 " { int pvi; for (pvi = 0; physvols[pvi] != NULL; ++pvi)\n"; + pr " RESOLVE_DEVICE (physvols[pvi], 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; | Bool n -> pr " %s = args.%s;\n" n n | Int n -> pr " %s = args.%s;\n" n n | FileIn _ | FileOut _ -> () @@ -4847,6 +4859,7 @@ and generate_daemon_actions () = pr "\n" ); + (* this is used at least for do_equal *) if List.exists (function Pathname _ -> true | _ -> false) (snd style) then ( (* Emit NEED_ROOT just once, even when there are two or @@ -5160,7 +5173,8 @@ static void print_error (guestfs_h *g, void *data, const char *msg) fprintf (stderr, \"%%s\\n\", msg); } -static void print_strings (char * const * const argv) +/* FIXME: nearly identical code appears in fish.c */ +static void print_strings (char const *const *argv) { int argc; @@ -5169,7 +5183,7 @@ static void print_strings (char * const * const argv) } /* -static void print_table (char * const * const argv) +static void print_table (char const *const *argv) { int i; @@ -5749,7 +5763,7 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = | Int _, _ | Bool _, _ | FileIn _, _ | FileOut _, _ -> () - | StringList n, arg -> + | StringList n, arg | DeviceList n, arg -> let strs = string_split " " arg in iteri ( fun i str -> @@ -5797,7 +5811,7 @@ and generate_test_command_call ?(expect_error = false) ?test test_name cmd = pr ", %s" n | FileIn _, arg | FileOut _, arg -> pr ", \"%s\"" (c_quote arg) - | StringList n, _ -> + | StringList n, _ | DeviceList n, _ -> pr ", %s" n | Int _, arg -> let i = @@ -6046,7 +6060,7 @@ and generate_fish_cmds () = | OptString n | FileIn n | FileOut n -> pr " const char *%s;\n" n - | StringList n -> pr " char **%s;\n" n + | StringList n | DeviceList n -> pr " char *const *%s;\n" n | Bool n -> pr " int %s;\n" n | Int n -> pr " int %s;\n" n ) (snd style); @@ -6073,7 +6087,7 @@ and generate_fish_cmds () = | FileOut name -> pr " %s = strcmp (argv[%d], \"-\") != 0 ? argv[%d] : \"/dev/stdout\";\n" name i i - | StringList name -> + | StringList name | DeviceList name -> pr " %s = parse_string_list (argv[%d]);\n" name i | Bool name -> pr " %s = is_true (argv[%d]) ? 1 : 0;\n" name i @@ -6298,7 +6312,7 @@ and generate_fish_actions_pod () = function | Pathname n | Device n | Dev_or_Path n | String n -> pr " %s" n | OptString n -> pr " %s" n - | StringList n -> pr " '%s ...'" n + | StringList n | DeviceList n -> pr " '%s ...'" n | Bool _ -> pr " true|false" | Int n -> pr " %s" n | FileIn n | FileOut n -> pr " (%s|-)" n @@ -6368,10 +6382,9 @@ and generate_prototype ?(extern = true) ?(static = false) ?(semicolon = true) | OptString n -> next (); pr "const char *%s" n - | StringList n -> + | StringList n | DeviceList n -> next (); - if not in_daemon then pr "char * const* const %s" n - else pr "char **%s" n + pr "char *const *%s" n | Bool n -> next (); pr "int %s" n | Int n -> next (); pr "int %s" n | FileIn n @@ -6635,7 +6648,7 @@ copy_table (char * const * argv) pr " const char *%s =\n" n; pr " %sv != Val_int (0) ? String_val (Field (%sv, 0)) : NULL;\n" n n - | StringList n -> + | StringList n | DeviceList n -> pr " char **%s = ocaml_guestfs_strings_val (g, %sv);\n" n n | Bool n -> pr " int %s = Bool_val (%sv);\n" n n @@ -6677,7 +6690,7 @@ copy_table (char * const * argv) List.iter ( function - | StringList n -> + | StringList n | DeviceList n -> pr " ocaml_guestfs_free_strings (%s);\n" n; | Pathname _ | Device _ | Dev_or_Path _ | String _ | OptString _ | Bool _ | Int _ | FileIn _ | FileOut _ -> () @@ -6765,7 +6778,7 @@ and generate_ocaml_prototype ?(is_external = false) name style = function | Pathname _ | Device _ | Dev_or_Path _ | String _ | FileIn _ | FileOut _ -> pr "string -> " | OptString _ -> pr "string option -> " - | StringList _ -> pr "string array -> " + | StringList _ | DeviceList _ -> pr "string array -> " | Bool _ -> pr "bool -> " | Int _ -> pr "int -> " ) (snd style); @@ -6916,7 +6929,7 @@ DESTROY (g) * 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 -> 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 ) (snd style); @@ -6926,7 +6939,7 @@ DESTROY (g) function | Pathname _ | Device _ | Dev_or_Path _ | String _ | OptString _ | Bool _ | Int _ | FileIn _ | FileOut _ -> () - | StringList n -> pr " free (%s);\n" n + | StringList n | DeviceList n -> pr " free (%s);\n" n ) (snd style) in @@ -7299,7 +7312,7 @@ and generate_perl_prototype name style = | Pathname n | Device n | Dev_or_Path n | String n | OptString n | Bool n | Int n | FileIn n | FileOut n -> pr "$%s" n - | StringList n -> + | StringList n | DeviceList n -> pr "\\@%s" n ) (snd style); pr ");" @@ -7339,11 +7352,11 @@ put_handle (guestfs_h *g) } /* This list should be freed (but not the strings) after use. */ -static const char ** +static char ** get_string_list (PyObject *obj) { int i, len; - const char **r; + char **r; assert (obj); @@ -7559,9 +7572,9 @@ py_guestfs_close (PyObject *self, PyObject *args) | Pathname n | Device n | Dev_or_Path n | String n | FileIn n | FileOut n -> pr " const char *%s;\n" n | OptString n -> pr " const char *%s;\n" n - | StringList n -> + | StringList n | DeviceList n -> pr " PyObject *py_%s;\n" n; - pr " const char **%s;\n" n + pr " char **%s;\n" n | Bool n -> pr " int %s;\n" n | Int n -> pr " int %s;\n" n ) (snd style); @@ -7574,7 +7587,7 @@ py_guestfs_close (PyObject *self, PyObject *args) function | Pathname _ | Device _ | Dev_or_Path _ | String _ | FileIn _ | FileOut _ -> pr "s" | OptString _ -> pr "z" - | StringList _ -> pr "O" + | StringList _ | DeviceList _ -> pr "O" | Bool _ -> pr "i" (* XXX Python has booleans? *) | Int _ -> pr "i" ) (snd style); @@ -7584,7 +7597,7 @@ py_guestfs_close (PyObject *self, PyObject *args) function | Pathname n | Device n | Dev_or_Path n | String n | FileIn n | FileOut n -> pr ", &%s" n | OptString n -> pr ", &%s" n - | StringList n -> pr ", &py_%s" n + | StringList n | DeviceList n -> pr ", &py_%s" n | Bool n -> pr ", &%s" n | Int n -> pr ", &%s" n ) (snd style); @@ -7597,7 +7610,7 @@ py_guestfs_close (PyObject *self, PyObject *args) function | Pathname _ | Device _ | Dev_or_Path _ | String _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ -> () - | StringList n -> + | StringList n | DeviceList n -> pr " %s = get_string_list (py_%s);\n" n n; pr " if (!%s) return NULL;\n" n ) (snd style); @@ -7612,7 +7625,7 @@ py_guestfs_close (PyObject *self, PyObject *args) function | Pathname _ | Device _ | Dev_or_Path _ | String _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ -> () - | StringList n -> + | StringList n | DeviceList n -> pr " free (%s);\n" n ) (snd style); @@ -7927,7 +7940,7 @@ static VALUE ruby_guestfs_close (VALUE gv) pr " \"%s\", \"%s\");\n" n name | OptString n -> pr " const char *%s = !NIL_P (%sv) ? StringValueCStr (%sv) : NULL;\n" n n n - | StringList n -> + | StringList n | DeviceList n -> pr " char **%s;\n" n; pr " Check_Type (%sv, T_ARRAY);\n" n; pr " {\n"; @@ -7973,7 +7986,7 @@ static VALUE ruby_guestfs_close (VALUE gv) function | Pathname _ | Device _ | Dev_or_Path _ | String _ | FileIn _ | FileOut _ | OptString _ | Bool _ | Int _ -> () - | StringList n -> + | StringList n | DeviceList n -> pr " free (%s);\n" n ) (snd style); @@ -8289,7 +8302,7 @@ and generate_java_prototype ?(public=false) ?(privat=false) ?(native=false) | FileIn n | FileOut n -> pr "String %s" n - | StringList n -> + | StringList n | DeviceList n -> pr "String[] %s" n | Bool n -> pr "boolean %s" n @@ -8408,7 +8421,7 @@ Java_com_redhat_et_libguestfs_GuestFS__1close | FileIn n | FileOut n -> pr ", jstring j%s" n - | StringList n -> + | StringList n | DeviceList n -> pr ", jobjectArray j%s" n | Bool n -> pr ", jboolean j%s" n @@ -8461,7 +8474,7 @@ Java_com_redhat_et_libguestfs_GuestFS__1close | FileIn n | FileOut n -> pr " const char *%s;\n" n - | StringList n -> + | StringList n | DeviceList n -> pr " int %s_len;\n" n; pr " const char **%s;\n" n | Bool n @@ -8495,7 +8508,7 @@ Java_com_redhat_et_libguestfs_GuestFS__1close * a NULL parameter. *) pr " %s = j%s ? (*env)->GetStringUTFChars (env, j%s, NULL) : NULL;\n" n n n - | StringList n -> + | StringList n | DeviceList n -> pr " %s_len = (*env)->GetArrayLength (env, j%s);\n" n n; pr " %s = guestfs_safe_malloc (g, sizeof (char *) * (%s_len+1));\n" n n; pr " for (i = 0; i < %s_len; ++i) {\n" n; @@ -8526,7 +8539,7 @@ Java_com_redhat_et_libguestfs_GuestFS__1close | OptString n -> pr " if (j%s)\n" n; pr " (*env)->ReleaseStringUTFChars (env, j%s, %s);\n" n n - | StringList n -> + | StringList n | DeviceList n -> pr " for (i = 0; i < %s_len; ++i) {\n" n; pr " jobject o = (*env)->GetObjectArrayElement (env, j%s, i);\n" n; @@ -8795,7 +8808,7 @@ last_error h = do | FileOut n | Pathname n | Device n | Dev_or_Path n | String n -> pr "withCString %s $ \\%s -> " n n | OptString n -> pr "maybeWith withCString %s $ \\%s -> " n n - | StringList n -> pr "withMany withCString %s $ \\%s -> withArray0 nullPtr %s $ \\%s -> " n n n n + | StringList n | DeviceList n -> pr "withMany withCString %s $ \\%s -> withArray0 nullPtr %s $ \\%s -> " n n n n | Bool _ | Int _ -> () ) (snd style); (* Convert integer arguments. *) @@ -8805,7 +8818,7 @@ last_error h = do | Bool n -> sprintf "(fromBool %s)" n | Int n -> sprintf "(fromIntegral %s)" n | FileIn n | FileOut n - | Pathname n | Device n | Dev_or_Path n | String n | OptString n | StringList n -> n + | Pathname n | Device n | Dev_or_Path n | String n | OptString n | StringList n | DeviceList n -> n ) (snd style) in pr "withForeignPtr h (\\p -> c_%s %s)\n" name (String.concat " " ("p" :: args)); @@ -8857,7 +8870,7 @@ and generate_haskell_prototype ~handle ?(hs = false) style = (match arg with | Pathname _ | Device _ | Dev_or_Path _ | String _ -> pr "%s" string | OptString _ -> if hs then pr "Maybe String" else pr "CString" - | StringList _ -> if hs then pr "[String]" else pr "Ptr CString" + | StringList _ | DeviceList _ -> if hs then pr "[String]" else pr "Ptr CString" | Bool _ -> pr "%s" bool | Int _ -> pr "%s" int | FileIn _ -> pr "%s" string @@ -8936,7 +8949,7 @@ print_strings (char * const* const argv) | FileIn n | FileOut n -> pr " printf (\"%%s\\n\", %s);\n" n | OptString n -> pr " printf (\"%%s\\n\", %s ? %s : \"null\");\n" n n - | StringList n -> pr " print_strings (%s);\n" n + | StringList n | DeviceList n -> pr " print_strings (%s);\n" n | Bool n -> pr " printf (\"%%s\\n\", %s ? \"true\" : \"false\");\n" n | Int n -> pr " printf (\"%%d\\n\", %s);\n" n ) (snd style); -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * src/generator.ml: Include <Python.h> *before* <stdio.h> to avoid redefinition warning about "_POSIX_C_SOURCE". --- src/generator.ml | 4 ++-- 1 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/generator.ml b/src/generator.ml index 8a8d66e..58c3c9c 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -7322,12 +7322,12 @@ and generate_python_c () = generate_header CStyle LGPLv2; pr "\ +#include <Python.h> + #include <stdio.h> #include <stdlib.h> #include <assert.h> -#include <Python.h> - #include \"guestfs.h\" typedef struct { -- 1.6.4.378.g88f2f

Richard W.M. Jones wrote: I think so, too.

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/command.c (do_sh_lines, do_sh): Do not shadow global "command". --- daemon/command.c | 8 ++++---- 1 files changed, 4 insertions(+), 4 deletions(-) diff --git a/daemon/command.c b/daemon/command.c index 9fbbd95..b2b5ef9 100644 --- a/daemon/command.c +++ b/daemon/command.c @@ -132,17 +132,17 @@ do_command_lines (char *const *argv) } char * -do_sh (const char *command) +do_sh (const char *cmd) { - const char *argv[] = { "/bin/sh", "-c", command, NULL }; + const char *argv[] = { "/bin/sh", "-c", cmd, NULL }; return do_command ((char **) argv); } char ** -do_sh_lines (const char *command) +do_sh_lines (const char *cmd) { - const char *argv[] = { "/bin/sh", "-c", command, NULL }; + const char *argv[] = { "/bin/sh", "-c", cmd, NULL }; return do_command_lines ((char **) argv); } -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/guestfsd.c (xread, xwrite): Use char* pointers instead. --- daemon/guestfsd.c | 6 ++++-- 1 files changed, 4 insertions(+), 2 deletions(-) diff --git a/daemon/guestfsd.c b/daemon/guestfsd.c index fad49fb..ad3ce15 100644 --- a/daemon/guestfsd.c +++ b/daemon/guestfsd.c @@ -273,9 +273,10 @@ sysroot_path (const char *path) } int -xwrite (int sock, const void *buf, size_t len) +xwrite (int sock, const void *v_buf, size_t len) { int r; + const char *buf = v_buf; while (len > 0) { r = write (sock, buf, len); @@ -291,9 +292,10 @@ xwrite (int sock, const void *buf, size_t len) } int -xread (int sock, void *buf, size_t len) +xread (int sock, void *v_buf, size_t len) { int r; + char *buf = v_buf; while (len > 0) { r = read (sock, buf, len); -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/zero.c (do_zero): Detect write and close errors. --- daemon/zero.c | 18 ++++++++++++++---- 1 files changed, 14 insertions(+), 4 deletions(-) diff --git a/daemon/zero.c b/daemon/zero.c index ce45ae2..0ade242 100644 --- a/daemon/zero.c +++ b/daemon/zero.c @@ -41,10 +41,20 @@ do_zero (const char *device) memset (buf, 0, sizeof buf); + int err = 0; + int saved_errno = 0; for (i = 0; i < 32; ++i) - (void) write (fd, buf, sizeof buf); - - close (fd); + if (write (fd, buf, sizeof buf) != sizeof buf) { + saved_errno = errno; + err = -1; + } + + if (close (fd) && saved_errno == 0) { + saved_errno = errno; + err = -1; + } - return 0; + if (saved_errno) + errno = saved_errno; + return err; } -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/wc.c (wc): Make "flag" param const. * daemon/blockdev.c (call_blockdev): Likewise, for "switc". --- daemon/blockdev.c | 2 +- daemon/wc.c | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/daemon/blockdev.c b/daemon/blockdev.c index e0ac350..1aa6eaa 100644 --- a/daemon/blockdev.c +++ b/daemon/blockdev.c @@ -32,7 +32,7 @@ * we centralize it in one call. */ static int64_t -call_blockdev (const char *device, char *switc, int extraarg, int prints) +call_blockdev (const char *device, const char *switc, int extraarg, int prints) { int r; int64_t rv; diff --git a/daemon/wc.c b/daemon/wc.c index 93d4ebb..b64940b 100644 --- a/daemon/wc.c +++ b/daemon/wc.c @@ -28,7 +28,7 @@ #include "actions.h" static int -wc (char *flag, const char *path) +wc (const char *flag, const char *path) { char *buf; char *out, *err; -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/fallocate.c (do_fallocate): Format was not a string literal. * daemon/sfdisk.c (sfdisk): Likewise. --- daemon/fallocate.c | 2 +- daemon/sfdisk.c | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/fallocate.c b/daemon/fallocate.c index eb84145..20a75e6 100644 --- a/daemon/fallocate.c +++ b/daemon/fallocate.c @@ -36,7 +36,7 @@ do_fallocate (const char *path, int len) fd = open (path, O_WRONLY | O_CREAT | O_TRUNC | O_NOCTTY, 0666); CHROOT_OUT; if (fd == -1) { - reply_with_perror (path); + reply_with_perror ("failed to open %s", path); return -1; } diff --git a/daemon/sfdisk.c b/daemon/sfdisk.c index 6d49387..b68b9f6 100644 --- a/daemon/sfdisk.c +++ b/daemon/sfdisk.c @@ -72,13 +72,13 @@ sfdisk (const char *device, int n, int cyls, int heads, int sectors, fp = popen (buf, "w"); if (fp == NULL) { - reply_with_perror (buf); + reply_with_perror ("failed to open pipe: %s", buf); return -1; } for (i = 0; lines[i] != NULL; ++i) { if (fprintf (fp, "%s\n", lines[i]) < 0) { - reply_with_perror (buf); + reply_with_perror ("failed to write to pipe: %s", buf); pclose (fp); return -1; } -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/daemon.h (main_loop): Use "noreturn" attribute. --- daemon/daemon.h | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/daemon.h b/daemon/daemon.h index 2a2b840..0ce56e1 100644 --- a/daemon/daemon.h +++ b/daemon/daemon.h @@ -91,7 +91,7 @@ extern guestfs_int_lvm_vg_list *parse_command_line_vgs (void); extern guestfs_int_lvm_lv_list *parse_command_line_lvs (void); /*-- in proto.c --*/ -extern void main_loop (int sock); +extern void main_loop (int sock) __attribute__((noreturn)); /* ordinary daemon functions use these to indicate errors */ extern void reply_with_error (const char *fs, ...) -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/proto.c (reply): Fix typo that would cause us to ignore failed write-to-socket. --- daemon/proto.c | 2 +- 1 files changed, 1 insertions(+), 1 deletions(-) diff --git a/daemon/proto.c b/daemon/proto.c index 709f978..9b33902 100644 --- a/daemon/proto.c +++ b/daemon/proto.c @@ -288,7 +288,7 @@ reply (xdrproc_t xdrp, char *ret) fprintf (stderr, "xwrite failed\n"); exit (1); } - if (xwrite (sock, buf, len) == len) { + if (xwrite (sock, buf, len) == -1) { fprintf (stderr, "xwrite failed\n"); exit (1); } -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/xattr.c (getxattrs): Use an unsigned index. --- daemon/xattr.c | 10 ++++++---- 1 files changed, 6 insertions(+), 4 deletions(-) diff --git a/daemon/xattr.c b/daemon/xattr.c index 17d9382..1531070 100644 --- a/daemon/xattr.c +++ b/daemon/xattr.c @@ -195,11 +195,13 @@ getxattrs (const char *path, error: free (buf); if (r) { - if (r->guestfs_int_xattr_list_val) - for (i = 0; i < r->guestfs_int_xattr_list_len; ++i) { - free (r->guestfs_int_xattr_list_val[i].attrname); - free (r->guestfs_int_xattr_list_val[i].attrval.attrval_val); + if (r->guestfs_int_xattr_list_val) { + unsigned int k; + for (k = 0; k < r->guestfs_int_xattr_list_len; ++k) { + free (r->guestfs_int_xattr_list_val[k].attrname); + free (r->guestfs_int_xattr_list_val[k].attrval.attrval_val); } + } free (r->guestfs_int_xattr_list_val); } free (r); -- 1.6.4.378.g88f2f

From: Jim Meyering <meyering(a)redhat.com&gt; * daemon/m4/gnulib-cache.m4: Add two modules: manywarnings, warnings. * daemon/configure.ac: Implement --enable-gcc-warnings, and selectively disable a few warning options that are either not useful or that provoke too many warnings for now. Define and AC_SUBST WARN_CFLAGS and WERROR_CFLAGS. * daemon/Makefile.am (guestfsd_CFLAGS): Use $(WARN_CFLAGS) and $(WERROR_CFLAGS), rather than just -Wall. --- daemon/Makefile.am | 3 +- daemon/configure.ac | 61 +++++++++++++++++++++++++++++++++++++++++++++ daemon/m4/gnulib-cache.m4 | 4 ++- 3 files changed, 66 insertions(+), 2 deletions(-) diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 9406944..224dfe1 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -80,5 +80,6 @@ guestfsd_SOURCES = \ $(top_builddir)/../src/guestfs_protocol.c AM_CPPFLAGS = -I$(srcdir)/lib -Ilib -guestfsd_CFLAGS = -Wall +guestfsd_CFLAGS = $(WARN_CFLAGS) $(WERROR_CFLAGS) + LDADD = lib/libgnu.a diff --git a/daemon/configure.ac b/daemon/configure.ac index b0c7f26..f2f96c2 100644 --- a/daemon/configure.ac +++ b/daemon/configure.ac @@ -48,6 +48,67 @@ AC_PROG_CPP gl_EARLY gl_INIT +AC_ARG_ENABLE([gcc-warnings], + [AS_HELP_STRING([--enable-gcc-warnings], + [turn on lots of GCC warnings (for developers)])], + [case $enableval in + yes|no) ;; + *) AC_MSG_ERROR([bad value $enableval for gcc-warnings option]) ;; + esac + gl_gcc_warnings=$enableval], + [gl_gcc_warnings=no] +) + +if test "$gl_gcc_warnings" = yes; then + gl_WARN_ADD([-Werror], [WERROR_CFLAGS]) + AC_SUBST([WERROR_CFLAGS]) + + nw= + # This, $nw, is the list of warnings we disable. + nw="$nw -Wdeclaration-after-statement" # too useful to forbid + nw="$nw -Waggregate-return" # anachronistic + nw="$nw -Wc++-compat" # We don't care about C++ compilers + nw="$nw -Wundef" # Warns on '#if GNULIB_FOO' etc in gnulib + nw="$nw -Wtraditional" # Warns on #elif which we use often + nw="$nw -Wcast-qual" # Too many warnings for now + nw="$nw -Wconversion" # Too many warnings for now + nw="$nw -Wsystem-headers" # Don't let system headers trigger warnings + nw="$nw -Wsign-conversion" # Too many warnings for now + nw="$nw -Wtraditional-conversion" # Too many warnings for now + nw="$nw -Wunreachable-code" # Too many warnings for now + nw="$nw -Wpadded" # Our structs are not padded + nw="$nw -Wredundant-decls" # openat.h declares e.g., mkdirat + nw="$nw -Wlogical-op" # any use of fwrite provokes this + nw="$nw -Wvla" # two warnings in mount.c + # things I might fix soon: + nw="$nw -Wmissing-format-attribute" # daemon.h's asprintf_nowarn + nw="$nw -Winline" # daemon.h's asprintf_nowarn + nw="$nw -Wshadow" # numerous, plus we're not unanimous + # ?? -Wstrict-overflow + + gl_MANYWARN_ALL_GCC([ws]) + gl_MANYWARN_COMPLEMENT([ws], [$ws], [$nw]) + for w in $ws; do + gl_WARN_ADD([$w]) + done + gl_WARN_ADD([-Wno-unused-parameter]) # stubs.c + gl_WARN_ADD([-Wno-jump-misses-init]) # stubs.c + gl_WARN_ADD([-Wno-unused-variable]) # FIXME: only temporary, for guestfs_protocol.c, etc + + # In spite of excluding -Wlogical-op above, it is enabled, as of + # gcc 4.5.0 20090517, and it provokes warnings in cat.c, dd.c, truncate.c + gl_WARN_ADD([-Wno-logical-op]) + + gl_WARN_ADD([-fdiagnostics-show-option]) + + AC_SUBST([WARN_CFLAGS]) + + AC_DEFINE([lint], [1], [Define to 1 if the compiler is checking for lint.]) + AC_DEFINE([_FORTIFY_SOURCE], [2], + [enable compile-time and run-time bounds-checking, and some warnings]) + AC_DEFINE([GNULIB_PORTCHECK], [1], [enable some gnulib portability checks]) +fi + AC_C_PROTOTYPES test "x$U" != "x" && AC_MSG_ERROR([Compiler not ANSI compliant]) diff --git a/daemon/m4/gnulib-cache.m4 b/daemon/m4/gnulib-cache.m4 index 8aa504a..ea499d8 100644 --- a/daemon/m4/gnulib-cache.m4 +++ b/daemon/m4/gnulib-cache.m4 @@ -15,12 +15,14 @@ # Specification in the form of a command-line invocation: -# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl hash +# gnulib-tool --import --dir=. --lib=libgnu --source-base=lib --m4-base=m4 --doc-base=doc --tests-base=tests --aux-dir=build-aux --with-tests --no-libtool --macro-prefix=gl hash manywarnings warnings # Specification in the form of a few gnulib-tool.m4 macro invocations: gl_LOCAL_DIR([]) gl_MODULES([ hash + manywarnings + warnings ]) gl_AVOID([]) gl_SOURCE_BASE([lib]) -- 1.6.4.378.g88f2f