total warning-removal for daemon/
The warnings in daemon were aggravating and risky for development (too easy to miss new ones) so I spent some time last week and today working on removing them. The first patch gets us down to almost no warnings with the original -Wall setting. That was by far the hardest part. Once I'd done that, I enabled nearly all of gcc's warnings via gnulib's warnings and manywarnings modules and the code added to configure.ac in the final patch. I did it that way so that we can still bisect without compile failure, even when using the new --enable-gcc-warnings option. I.e., fix all warnings, *and then* add the code that turns on the new -W options along with -Werror. I suggest that developers always configure with --enable-gcc-warnings. The 2nd patch is self explanatory. 0003 adds some omitted "\n"-after ";" in emitted code. The remaining changes address new warnings exposed by using all of the new -W options. Among them, there were some real bugs. 0006 is a portability bug, at least. 0007 probably doesn't matter much in practice, but I looked because of a warning about ignored write return value. 0011 is definitely a bug. It was highlighted via the sign-vs-unsigned warning. 751 0001-adjust-const-pointers-to-avoid-warnings.patch 33 0002-python-avoid-_POSIX_C_SOURCE-redefinition-warning.patch 39 0003-generator.ml-emit-slightly-prettier-code.patch 50 0004-avoid-warning-about-old-style-no-param-function-defi.patch 39 0005-command.c-avoid-shadowing-a-global-function.patch 41 0006-guestfsd.c-don-t-perform-arithmetic-on-void-pointers.patch 42 0007-daemon-zero-don-t-ignore-write-and-close-errors.patch 41 0008-wc-blockdev-avoid-warnings-about-discarding-const-qu.patch 48 0009-sfdisk.c-fallocate.c-use-a-string-literal-as-format.patch 26 0010-daemon.h-avoid-warning-about-possible-noreturn-funct.patch 27 0011-guestfsd-don-t-ignore-failed-write-to-socket.patch 35 0012-xattr.c-avoid-warning-about-comparison-between-signe.patch 125 0013-daemon-enable-Werror-and-many-gcc-warnings-when-enab.patch $ diffstat 00* b/daemon/Makefile.am | 3 - b/daemon/augeas.c | 2 b/daemon/blockdev.c | 2 b/daemon/command.c | 12 ++-- b/daemon/configure.ac | 61 ++++++++++++++++++++++++ b/daemon/daemon.h | 6 +- b/daemon/debug.c | 2 b/daemon/df.c | 4 - b/daemon/fallocate.c | 2 b/daemon/guestfsd.c | 20 ++++---- b/daemon/lvm.c | 12 +--- b/daemon/m4/gnulib-cache.m4 | 5 +- b/daemon/proto.c | 3 - b/daemon/sfdisk.c | 10 ++-- b/daemon/sync.c | 3 - b/daemon/wc.c | 3 - b/daemon/xattr.c | 11 ++-- b/daemon/zero.c | 19 +++++-- b/fish/fish.c | 6 +- b/fish/fish.h | 6 +- b/src/generator.ml | 108 ++++++++++++++++++++++++-------------------- daemon/blockdev.c | 2 daemon/command.c | 9 +-- daemon/daemon.h | 3 - daemon/guestfsd.c | 7 +- daemon/sfdisk.c | 5 -- src/generator.ml | 12 ++-- 27 files changed, 206 insertions(+), 132 deletions(-)
From: Jim Meyering <meyering@redhat.com> --- 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
On Mon, Aug 17, 2009 at 11:10:52AM +0200, Jim Meyering wrote: [...] ACK. Addition of DeviceList is a good change. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
Richard W.M. Jones wrote:
On Mon, Aug 17, 2009 at 11:10:52AM +0200, Jim Meyering wrote: [...]
ACK. Addition of DeviceList is a good change.
Thanks for the quick reviews. I've adjusted that log, rebased against the lastest on master and have just pushed, even though "make check" hasn't yet completed.
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:10:53AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* 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
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
ACK. (Really a bug in python headers, I guess ...) Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-p2v converts physical machines to virtual machines. Boot with a live CD or over the network (PXE) and turn machines into Xen guests. http://et.redhat.com/~rjones/virt-p2v
Richard W.M. Jones wrote:
On Mon, Aug 17, 2009 at 11:10:53AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* 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
_______________________________________________ Libguestfs mailing list Libguestfs@redhat.com https://www.redhat.com/mailman/listinfo/libguestfs
ACK.
(Really a bug in python headers, I guess ...)
I think so, too.
From: Jim Meyering <meyering@redhat.com> * src/generator.ml: Emit a few omitted newlines. --- src/generator.ml | 6 +++--- 1 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/generator.ml b/src/generator.ml index 58c3c9c..5ca0f30 100755 --- a/src/generator.ml +++ b/src/generator.ml @@ -4837,10 +4837,10 @@ and generate_daemon_actions () = pr " ABS_PATH (%s, goto done);\n" n; | Device n -> pr_args n; - pr " RESOLVE_DEVICE (%s, goto done);" n; + pr " RESOLVE_DEVICE (%s, goto done);\n" n; | Dev_or_Path n -> pr_args n; - pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);" n; + pr " REQUIRE_ROOT_OR_RESOLVE_DEVICE (%s, goto done);\n" n; | String n -> pr_args n | OptString n -> pr " %s = args.%s ? *args.%s : NULL;\n" n n n | StringList n -> @@ -7519,7 +7519,7 @@ py_guestfs_close (PyObject *self, PyObject *args) typ name; pr " else {\n"; pr " Py_INCREF (Py_None);\n"; - pr " PyDict_SetItemString (dict, \"%s\", Py_None);" name; + pr " PyDict_SetItemString (dict, \"%s\", Py_None);\n" name; pr " }\n" | name, FChar -> pr " PyDict_SetItemString (dict, \"%s\",\n" name; -- 1.6.4.378.g88f2f
On Mon, Aug 17, 2009 at 11:10:54AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* src/generator.ml: Emit a few omitted newlines.
ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
From: Jim Meyering <meyering@redhat.com> * daemon/df.c (do_df, do_df_h): Add "void". * sync.c (do_sync): Likewise. --- daemon/df.c | 4 ++-- daemon/sync.c | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/daemon/df.c b/daemon/df.c index ad662fa..563760d 100644 --- a/daemon/df.c +++ b/daemon/df.c @@ -28,7 +28,7 @@ #include "actions.h" char * -do_df () +do_df (void) { int r; char *out, *err; @@ -49,7 +49,7 @@ do_df () } char * -do_df_h () +do_df_h (void) { int r; char *out, *err; diff --git a/daemon/sync.c b/daemon/sync.c index b353b6c..e96bf03 100644 --- a/daemon/sync.c +++ b/daemon/sync.c @@ -24,7 +24,7 @@ #include "actions.h" int -do_sync () +do_sync (void) { sync (); return 0; -- 1.6.4.378.g88f2f
On Mon, Aug 17, 2009 at 11:10:55AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/df.c (do_df, do_df_h): Add "void". * sync.c (do_sync): Likewise.
I thought it could be omitted for function definitions (not for forward declarations)? Anyhow, ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
Richard W.M. Jones wrote:
On Mon, Aug 17, 2009 at 11:10:55AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/df.c (do_df, do_df_h): Add "void". * sync.c (do_sync): Likewise.
I thought it could be omitted for function definitions (not for forward declarations)? Anyhow, ACK.
You're right that "void" may be omitted, in general. I've added them here solely to avoid noise from one of gcc's old-style warning options.
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:10:56AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/command.c (do_sh_lines, do_sh): Do not shadow global "command".
ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones libguestfs lets you edit virtual machines. Supports shell scripting, bindings from many languages. http://et.redhat.com/~rjones/libguestfs/ See what it can do: http://et.redhat.com/~rjones/libguestfs/recipes.html
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:10:57AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/guestfsd.c (xread, xwrite): Use char* pointers instead.
Bit of a strange change, but ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:10:58AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/zero.c (do_zero): Detect write and close errors.
ACK, good catch. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 75 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:10:59AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/wc.c (wc): Make "flag" param const. * daemon/blockdev.c (call_blockdev): Likewise, for "switc".
ACK. I guess these were char * because that's how the daemon code previously declared strings, before your other set of changes. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:11:00AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/fallocate.c (do_fallocate): Format was not a string literal. * daemon/sfdisk.c (sfdisk): Likewise.
Ooops, ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-df lists disk usage of guests without needing to install any software inside the virtual machine. Supports Linux and Windows. http://et.redhat.com/~rjones/virt-df/
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:11:01AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/daemon.h (main_loop): Use "noreturn" attribute.
ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:11:02AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/proto.c (reply): Fix typo that would cause us to ignore failed write-to-socket.
Nasty, ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:11:03AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* daemon/xattr.c (getxattrs): Use an unsigned index.
ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones Read my programming blog: http://rwmj.wordpress.com Fedora now supports 75 OCaml packages (the OPEN alternative to F#) http://cocan.org/getting_started_with_ocaml_on_red_hat_and_fedora
From: Jim Meyering <meyering@redhat.com> * 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
On Mon, Aug 17, 2009 at 11:11:04AM +0200, Jim Meyering wrote:
From: Jim Meyering <meyering@redhat.com>
* 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.
Good change, ACK. Rich. -- Richard Jones, Emerging Technologies, Red Hat http://et.redhat.com/~rjones virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://et.redhat.com/~rjones/virt-top
participants (2)
-
Jim Meyering -
Richard W.M. Jones