[PATCH 00/11] Various Coverity fixes
by Pino Toscano
Hi,
this patch series fixes some issues discovered by Coverity.
Most of them are memory leaks, usually on error; there are also invalid
memory access issues.
Thanks,
Pino Toscano (11):
java: link libguestfs_jni against libutils
java: fix invalid memory access for FBuffer in struct lists
daemon: tsk: properly use GUESTFS_MAX_CHUNK_SIZE
edit: fix small memory leak on error
java: fix possible memory leak on error
fish: fully init the msghdr buffers
tail: pass the right path for Windows guests
ocaml: do not try to malloc 0 elements in get_all_event_callbacks
python: do not try to malloc 0 elements in get_all_event_callbacks
ruby: do not try to malloc 0 elements in get_all_event_callbacks
java: do not try to malloc 0 elements in get_all_event_callbacks
cat/tail.c | 2 +-
common/edit/file-edit.c | 1 +
daemon/sleuthkit.c | 2 +-
fish/rc.c | 9 +++------
generator/java.ml | 8 +++-----
java/Makefile.am | 4 +++-
java/handle.c | 3 ++-
ocaml/guestfs-c.c | 3 ++-
python/handle.c | 3 ++-
ruby/ext/guestfs/handle.c | 3 ++-
10 files changed, 20 insertions(+), 18 deletions(-)
--
2.9.3
7 years, 2 months
[PATCH libguestfs] Use AC_HEADER_MAJOR to find definitions of major, minor, makedev.
by Richard W.M. Jones
Note this requires either the following fix in autoconf:
http://git.savannah.gnu.org/cgit/autoconf.git/commit/?id=e17a30e98
OR gnulib sys_types module plus gnulib
commit a512e041120e9012e69afa2f5c3adc196ec4999a (any gnulib more
recent than Sep 2016) which corrects the AC_HEADER_MAJOR macro in a
similar way.
---
bootstrap | 1 +
cat/ls.c | 7 +++++++
daemon/mknod.c | 6 ++++++
diff/diff.c | 7 +++++++
lib/fuse.c | 6 ++++++
m4/guestfs_libraries.m4 | 3 +++
mllib/unix_utils-c.c | 8 +++++++-
p2v/main.c | 6 ++++++
8 files changed, 43 insertions(+), 1 deletion(-)
diff --git a/bootstrap b/bootstrap
index 037d07e..faa10a3 100755
--- a/bootstrap
+++ b/bootstrap
@@ -92,6 +92,7 @@ strerror
strndup
symlinkat
sys_select
+sys_types
sys_wait
vasprintf
vc-list-files
diff --git a/cat/ls.c b/cat/ls.c
index e0b5ff8..7568a5f 100644
--- a/cat/ls.c
+++ b/cat/ls.c
@@ -31,7 +31,14 @@
#include <assert.h>
#include <time.h>
#include <libintl.h>
+
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
+#else
+#include <sys/types.h>
+#endif
#include "human.h"
#include "getprogname.h"
diff --git a/daemon/mknod.c b/daemon/mknod.c
index f8640ea..35dad15 100644
--- a/daemon/mknod.c
+++ b/daemon/mknod.c
@@ -25,7 +25,13 @@
#include <fcntl.h>
#include <sys/types.h>
#include <sys/stat.h>
+
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
+/* else it's in sys/types.h, included above */
+#endif
#include "guestfs_protocol.h"
#include "daemon.h"
diff --git a/diff/diff.c b/diff/diff.c
index fc31ad3..8c045ad 100644
--- a/diff/diff.c
+++ b/diff/diff.c
@@ -32,7 +32,14 @@
#include <time.h>
#include <libintl.h>
#include <sys/wait.h>
+
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
+#else
+#include <sys/types.h>
+#endif
#include "c-ctype.h"
#include "human.h"
diff --git a/lib/fuse.c b/lib/fuse.c
index 98bbc8d..7f80c49 100644
--- a/lib/fuse.c
+++ b/lib/fuse.c
@@ -26,7 +26,13 @@
#include <sys/wait.h>
#include <string.h>
#include <libintl.h>
+
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
+/* else it's in sys/types.h, included above */
+#endif
#if HAVE_FUSE
/* See <attr/xattr.h> */
diff --git a/m4/guestfs_libraries.m4 b/m4/guestfs_libraries.m4
index 0882d27..c265b29 100644
--- a/m4/guestfs_libraries.m4
+++ b/m4/guestfs_libraries.m4
@@ -82,6 +82,9 @@ AC_CHECK_FUNCS([\
statvfs \
sync])
+dnl Which header file defines major, minor, makedev.
+AC_HEADER_MAJOR
+
dnl Check for UNIX_PATH_MAX, creating a custom one if not available.
AC_MSG_CHECKING([for UNIX_PATH_MAX])
AC_COMPILE_IFELSE([
diff --git a/mllib/unix_utils-c.c b/mllib/unix_utils-c.c
index 1a4c3d7..f5aaaf6 100644
--- a/mllib/unix_utils-c.c
+++ b/mllib/unix_utils-c.c
@@ -27,9 +27,15 @@
#include <fnmatch.h>
#include <errno.h>
#include <sys/types.h>
-#include <sys/sysmacros.h>
#include <sys/statvfs.h>
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
+#include <sys/sysmacros.h>
+/* else it's in sys/types.h, included above */
+#endif
+
#include <caml/alloc.h>
#include <caml/fail.h>
#include <caml/memory.h>
diff --git a/p2v/main.c b/p2v/main.c
index 42d3bbb..af14240 100644
--- a/p2v/main.c
+++ b/p2v/main.c
@@ -32,7 +32,13 @@
#include <libintl.h>
#include <sys/types.h>
#include <sys/stat.h>
+
+#if MAJOR_IN_MKDEV
+#include <sys/mkdev.h>
+#elif MAJOR_IN_SYSMACROS
#include <sys/sysmacros.h>
+/* else it's in sys/types.h, included above */
+#endif
/* errors in <gtk.h> */
#pragma GCC diagnostic push
--
2.9.3
7 years, 2 months
[PATCH] erlang: Rename 'message' to something less generic.
by Richard W.M. Jones
It's not possible to define an action which takes a parameter called
'message' because the Erlang bindings use that as the name of an
internal variable. Solve this by renaming the Erlang internal
variable.
Fixes commit 84763d7fca3668c62ee3fe53d0e00a5a672f687b.
---
generator/erlang.ml | 14 +++++++-------
1 file changed, 7 insertions(+), 7 deletions(-)
diff --git a/generator/erlang.ml b/generator/erlang.ml
index 3753835..d0d2198 100644
--- a/generator/erlang.ml
+++ b/generator/erlang.ml
@@ -192,7 +192,7 @@ and generate_erlang_actions_h () =
extern guestfs_h *g;
-extern ETERM *dispatch (ETERM *message);
+extern ETERM *dispatch (ETERM *args_tuple);
extern int atom_equals (ETERM *atom, const char *name);
extern ETERM *make_error (const char *funname);
extern ETERM *unknown_optarg (const char *funname, ETERM *optargname);
@@ -205,7 +205,7 @@ extern int get_bool (ETERM *term);
extern int get_int (ETERM *term);
extern int64_t get_int64 (ETERM *term);
-#define ARG(i) (ERL_TUPLE_ELEMENT(message,(i)+1))
+#define ARG(i) (ERL_TUPLE_ELEMENT(args_tuple,(i)+1))
";
@@ -229,7 +229,7 @@ extern int64_t get_int64 (ETERM *term);
List.iter (
fun { name = name } ->
- pr "ETERM *run_%s (ETERM *message);\n" name
+ pr "ETERM *run_%s (ETERM *args_tuple);\n" name
) (actions |> external_functions |> sort);
pr "\n";
@@ -355,7 +355,7 @@ instead of erl_interface.
c_function = c_function; c_optarg_prefix = c_optarg_prefix } ->
pr "\n";
pr "ETERM *\n";
- pr "run_%s (ETERM *message)\n" name;
+ pr "run_%s (ETERM *args_tuple)\n" name;
pr "{\n";
iteri (
@@ -546,11 +546,11 @@ instead of erl_interface.
#include \"actions.h\"
ETERM *
-dispatch (ETERM *message)
+dispatch (ETERM *args_tuple)
{
ETERM *fun;
- fun = ERL_TUPLE_ELEMENT (message, 0);
+ fun = ERL_TUPLE_ELEMENT (args_tuple, 0);
/* XXX We should use gperf here. */
";
@@ -558,7 +558,7 @@ dispatch (ETERM *message)
List.iter (
fun { name = name; style = ret, args, optargs } ->
pr "if (atom_equals (fun, \"%s\"))\n" name;
- pr " return run_%s (message);\n" name;
+ pr " return run_%s (args_tuple);\n" name;
pr " else ";
) (actions |> external_functions |> sort);
--
2.9.3
7 years, 2 months
[PATCH] generator: Move some deprecated functions to actions_core_deprecated.ml.
by Richard W.M. Jones
Fixes commit 97773d2bbee8e28830fd689deef9e9f63ce0c18e.
---
generator/actions_core.ml | 83 ------------------------------------
generator/actions_core_deprecated.ml | 83 ++++++++++++++++++++++++++++++++++++
2 files changed, 83 insertions(+), 83 deletions(-)
diff --git a/generator/actions_core.ml b/generator/actions_core.ml
index e662827..ed89f74 100644
--- a/generator/actions_core.ml
+++ b/generator/actions_core.ml
@@ -1595,38 +1595,6 @@ Note that this call does not add the new disk to the handle. You
may need to call C<guestfs_add_drive_opts> separately." };
{ defaults with
- name = "stat"; added = (1, 9, 2);
- style = RStruct ("statbuf", "stat"), [Pathname "path"], [];
- deprecated_by = Some "statns";
- tests = [
- InitISOFS, Always, TestResult (
- [["stat"; "/empty"]], "ret->size == 0"), []
- ];
- shortdesc = "get file information";
- longdesc = "\
-Returns file information for the given C<path>.
-
-This is the same as the L<stat(2)> system call." };
-
- { defaults with
- name = "lstat"; added = (1, 9, 2);
- style = RStruct ("statbuf", "stat"), [Pathname "path"], [];
- deprecated_by = Some "lstatns";
- tests = [
- InitISOFS, Always, TestResult (
- [["lstat"; "/empty"]], "ret->size == 0"), []
- ];
- shortdesc = "get file information for a symbolic link";
- longdesc = "\
-Returns file information for the given C<path>.
-
-This is the same as C<guestfs_stat> except that if C<path>
-is a symbolic link, then the link is stat-ed, not the file it
-refers to.
-
-This is the same as the L<lstat(2)> system call." };
-
- { defaults with
name = "c_pointer"; added = (1, 29, 17);
style = RInt64 "ptr", [], [];
fish_output = Some FishOutputHexadecimal;
@@ -2259,28 +2227,6 @@ This creates an LVM logical volume called C<logvol>
on the volume group C<volgroup>, with C<size> megabytes." };
{ defaults with
- name = "write_file"; added = (0, 0, 8);
- style = RErr, [Pathname "path"; String "content"; Int "size"], [];
- protocol_limit_warning = true; deprecated_by = Some "write";
- (* Regression test for RHBZ#597135. *)
- tests = [
- InitScratchFS, Always, TestLastFail
- [["write_file"; "/write_file"; "abc"; "10000"]], []
- ];
- shortdesc = "create a file";
- longdesc = "\
-This call creates a file called C<path>. The contents of the
-file is the string C<content> (which can contain any 8 bit data),
-with length C<size>.
-
-As a special case, if C<size> is C<0>
-then the length is calculated using C<strlen> (so in this case
-the content cannot contain embedded ASCII NULs).
-
-I<NB.> Owing to a bug, writing content containing ASCII NUL
-characters does I<not> work, even if the length is specified." };
-
- { defaults with
name = "umount"; added = (0, 0, 8);
style = RErr, [Dev_or_Path "pathordevice"], [OBool "force"; OBool "lazyunmount"];
fish_alias = ["unmount"];
@@ -5479,26 +5425,6 @@ calls to associate logical volumes and volume groups.
See also C<guestfs_vgpvuuids>." };
{ defaults with
- name = "copy_size"; added = (1, 0, 87);
- style = RErr, [Dev_or_Path "src"; Dev_or_Path "dest"; Int64 "size"], [];
- progress = true; deprecated_by = Some "copy_device_to_device";
- tests = [
- InitScratchFS, Always, TestResult (
- [["mkdir"; "/copy_size"];
- ["write"; "/copy_size/src"; "hello, world"];
- ["copy_size"; "/copy_size/src"; "/copy_size/dest"; "5"];
- ["read_file"; "/copy_size/dest"]],
- "compare_buffers (ret, size, \"hello\", 5) == 0"), []
- ];
- shortdesc = "copy size bytes from source to destination using dd";
- longdesc = "\
-This command copies exactly C<size> bytes from one source device
-or file C<src> to another destination device or file C<dest>.
-
-Note this will fail if the source is too short or if the destination
-is not large enough." };
-
- { defaults with
name = "zero_device"; added = (1, 3, 1);
style = RErr, [Device "device"], [];
progress = true;
@@ -5785,15 +5711,6 @@ This command is the same as C<guestfs_pvresize> except that it
allows you to specify the new size (in bytes) explicitly." };
{ defaults with
- name = "ntfsresize_size"; added = (1, 3, 14);
- style = RErr, [Device "device"; Int64 "size"], [];
- optional = Some "ntfsprogs"; deprecated_by = Some "ntfsresize";
- shortdesc = "resize an NTFS filesystem (with size)";
- longdesc = "\
-This command is the same as C<guestfs_ntfsresize> except that it
-allows you to specify the new size (in bytes) explicitly." };
-
- { defaults with
name = "available_all_groups"; added = (1, 3, 15);
style = RStringList "groups", [], [];
tests = [
diff --git a/generator/actions_core_deprecated.ml b/generator/actions_core_deprecated.ml
index 3c9b890..b8cca79 100644
--- a/generator/actions_core_deprecated.ml
+++ b/generator/actions_core_deprecated.ml
@@ -105,6 +105,38 @@ list a directory contents without making many round-trips.
See also C<guestfs_lxattrlist> for a similarly efficient call
for getting extended attributes." };
+ { defaults with
+ name = "stat"; added = (1, 9, 2);
+ style = RStruct ("statbuf", "stat"), [Pathname "path"], [];
+ deprecated_by = Some "statns";
+ tests = [
+ InitISOFS, Always, TestResult (
+ [["stat"; "/empty"]], "ret->size == 0"), []
+ ];
+ shortdesc = "get file information";
+ longdesc = "\
+Returns file information for the given C<path>.
+
+This is the same as the L<stat(2)> system call." };
+
+ { defaults with
+ name = "lstat"; added = (1, 9, 2);
+ style = RStruct ("statbuf", "stat"), [Pathname "path"], [];
+ deprecated_by = Some "lstatns";
+ tests = [
+ InitISOFS, Always, TestResult (
+ [["lstat"; "/empty"]], "ret->size == 0"), []
+ ];
+ shortdesc = "get file information for a symbolic link";
+ longdesc = "\
+Returns file information for the given C<path>.
+
+This is the same as C<guestfs_stat> except that if C<path>
+is a symbolic link, then the link is stat-ed, not the file it
+refers to.
+
+This is the same as the L<lstat(2)> system call." };
+
]
let daemon_functions = [
@@ -751,4 +783,55 @@ List the files in F<directory> in the format of 'ls -laZ'.
This command is mostly useful for interactive sessions. It
is I<not> intended that you try to parse the output string." };
+ { defaults with
+ name = "write_file"; added = (0, 0, 8);
+ style = RErr, [Pathname "path"; String "content"; Int "size"], [];
+ protocol_limit_warning = true; deprecated_by = Some "write";
+ (* Regression test for RHBZ#597135. *)
+ tests = [
+ InitScratchFS, Always, TestLastFail
+ [["write_file"; "/write_file"; "abc"; "10000"]], []
+ ];
+ shortdesc = "create a file";
+ longdesc = "\
+This call creates a file called C<path>. The contents of the
+file is the string C<content> (which can contain any 8 bit data),
+with length C<size>.
+
+As a special case, if C<size> is C<0>
+then the length is calculated using C<strlen> (so in this case
+the content cannot contain embedded ASCII NULs).
+
+I<NB.> Owing to a bug, writing content containing ASCII NUL
+characters does I<not> work, even if the length is specified." };
+
+ { defaults with
+ name = "copy_size"; added = (1, 0, 87);
+ style = RErr, [Dev_or_Path "src"; Dev_or_Path "dest"; Int64 "size"], [];
+ progress = true; deprecated_by = Some "copy_device_to_device";
+ tests = [
+ InitScratchFS, Always, TestResult (
+ [["mkdir"; "/copy_size"];
+ ["write"; "/copy_size/src"; "hello, world"];
+ ["copy_size"; "/copy_size/src"; "/copy_size/dest"; "5"];
+ ["read_file"; "/copy_size/dest"]],
+ "compare_buffers (ret, size, \"hello\", 5) == 0"), []
+ ];
+ shortdesc = "copy size bytes from source to destination using dd";
+ longdesc = "\
+This command copies exactly C<size> bytes from one source device
+or file C<src> to another destination device or file C<dest>.
+
+Note this will fail if the source is too short or if the destination
+is not large enough." };
+
+ { defaults with
+ name = "ntfsresize_size"; added = (1, 3, 14);
+ style = RErr, [Device "device"; Int64 "size"], [];
+ optional = Some "ntfsprogs"; deprecated_by = Some "ntfsresize";
+ shortdesc = "resize an NTFS filesystem (with size)";
+ longdesc = "\
+This command is the same as C<guestfs_ntfsresize> except that it
+allows you to specify the new size (in bytes) explicitly." };
+
]
--
2.9.3
7 years, 2 months
[PATCH 0/5] Fix virt-rescue.
by Richard W.M. Jones
This fixes the main issues in virt-rescue and is usable.
There are some enhancements which could be made (in follow up work):
- An escape sequence and escape commands that could be handled by
virt-rescue, eg. to shut down the appliance, mount or unmount
filesystems.
- `virt-rescue -i' could be implemented cleanly by performing the
right API calls before handing control to the rescue shell.
Rich.
7 years, 2 months
[PATCH] parted: add more udev_settle calls.
by Dawid Zamirski
add udev_settle calls to print_partition_table and
sgdisk_info_extract_field because the inspect-os calls
guestfs_part_get_parttype and guestfs_part_get_gpt_guid for all
parition devices found and this causes intermittent with opening block
devices that are certainly present yet RESOLVE_DEVICE macro would fail
wiht ENOENT.
---
daemon/parted.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/daemon/parted.c b/daemon/parted.c
index 124d1e9..03e83cb 100644
--- a/daemon/parted.c
+++ b/daemon/parted.c
@@ -320,6 +320,8 @@ print_partition_table (const char *device, bool add_m_option)
CLEANUP_FREE char *err = NULL;
int r;
+ udev_settle ();
+
if (add_m_option)
r = command (&out, &err, str_parted, "-m", "-s", "--", device,
"unit", "b",
@@ -328,6 +330,9 @@ print_partition_table (const char *device, bool add_m_option)
r = command (&out, &err, str_parted, "-s", "--", device,
"unit", "b",
"print", NULL);
+
+ udev_settle ();
+
if (r == -1) {
int errcode = 0;
@@ -665,6 +670,8 @@ sgdisk_info_extract_field (const char *device, int partnum, const char *field,
return NULL;
}
+ udev_settle ();
+
CLEANUP_FREE char *err = NULL;
int r = commandf (NULL, &err, COMMAND_FLAG_FOLD_STDOUT_ON_STDERR,
str_sgdisk, device, "-i", partnum_str, NULL);
@@ -674,6 +681,8 @@ sgdisk_info_extract_field (const char *device, int partnum, const char *field,
return NULL;
}
+ udev_settle ();
+
CLEANUP_FREE_STRING_LIST char **lines = split_lines (err);
if (lines == NULL) {
reply_with_error ("'%s %s -i %i' returned no output",
--
2.9.3
7 years, 2 months
[PATCH WIP 0/5] Fix virt-rescue.
by Richard W.M. Jones
This set of patches fixes virt-rescue rather cleanly. In particular
the problems with handling ^C are completely fixed.
Work still to be done before this can go upstream:
- Shutdown doesn't work properly if you exit the shell. At the
moment to exit you must do 'reboot -f'.
Future improvements:
- An escape sequence and escape commands that could be handled by
virt-rescue, eg. to shut down the appliance, mount or unmount
filesystems.
- `virt-rescue -i' could be implemented cleanly by performing the
right API calls before handling control to the rescue shell.
I'm not intending the do the future stuff just yet.
Rich.
7 years, 2 months