The JNI library uses CLEANUP_FREE macros, whose functions are built in the internal libutils. Currently, trying to use functions that use CLEANUP_FREE variables will cause the java execution to stop with a symbol lookup error (for guestfs_int_cleanup_free). --- java/Makefile.am | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/java/Makefile.am b/java/Makefile.am index 06d0fc7..ea64f55 100644 --- a/java/Makefile.am +++ b/java/Makefile.am @@ -115,7 +115,9 @@ libguestfs_jni_la_CFLAGS = \ $(WARN_CFLAGS) $(WERROR_CFLAGS) \ $(JNI_CFLAGS) -libguestfs_jni_la_LIBADD = $(top_builddir)/lib/libguestfs.la +libguestfs_jni_la_LIBADD = \ + $(top_builddir)/common/utils/libutils.la \ + $(top_builddir)/lib/libguestfs.la libguestfs_jni_la_LDFLAGS = -version-info $(JNI_VERSION_INFO) -shared -- 2.9.3

Pass to fread the size of the buffer allocated, so it can be filled completely. This is even faster than reading only 4/8 bytes each iteration. --- daemon/sleuthkit.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/daemon/sleuthkit.c b/daemon/sleuthkit.c index e642731..bdbdb0f 100644 --- a/daemon/sleuthkit.c +++ b/daemon/sleuthkit.c @@ -122,7 +122,7 @@ send_command_output (const char *cmd) /* Send reply message before the file content. */ reply (NULL, NULL); - while ((ret = fread (buffer, 1, sizeof buffer, fp)) > 0) { + while ((ret = fread (buffer, 1, GUESTFS_MAX_CHUNK_SIZE, fp)) > 0) { ret = send_file_write (buffer, ret); if (ret < 0) { pclose (fp); -- 2.9.3

Use CLEANUP_FREE to properly dispose the temporary array used for StringList, DeviceList, FilenameList, and OStringList parameters: this way, an early return (jumping to the ret_error label) will not forget cleaning them up. --- generator/java.ml | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/generator/java.ml b/generator/java.ml index 2606f97..66729c6 100644 --- a/generator/java.ml +++ b/generator/java.ml @@ -715,7 +715,7 @@ throw_out_of_memory (JNIEnv *env, const char *msg) pr " size_t %s_size;\n" n | StringList n | DeviceList n | FilenameList n -> pr " size_t %s_len;\n" n; - pr " char **%s;\n" n + pr " CLEANUP_FREE char **%s = NULL;\n" n | Bool n | Int n -> pr " int %s;\n" n @@ -734,7 +734,7 @@ throw_out_of_memory (JNIEnv *env, const char *msg) | OBool _ | OInt _ | OInt64 _ | OString _ -> () | OStringList n -> pr " size_t %s_len;\n" n; - pr " char **%s;\n" n + pr " CLEANUP_FREE char **%s = NULL;\n" n ) optargs ); @@ -857,7 +857,6 @@ throw_out_of_memory (JNIEnv *env, const char *msg) n; pr " (*env)->ReleaseStringUTFChars (env, o, %s[i]);\n" n; pr " }\n"; - pr " free (%s);\n" n | Bool _ | Int _ | Int64 _ @@ -875,7 +874,6 @@ throw_out_of_memory (JNIEnv *env, const char *msg) n; pr " (*env)->ReleaseStringUTFChars (env, o, optargs_s.%s[i]);\n" n; pr " }\n"; - pr " free (%s);\n" n ) optargs; pr "\n"; -- 2.9.3

Call windows_path with the actual path to resolve, instead of a null pointer ('filename' is still null at that point), so Windows paths can be properly resolved. --- cat/tail.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cat/tail.c b/cat/tail.c index 8785d45..51da5fc 100644 --- a/cat/tail.c +++ b/cat/tail.c @@ -309,7 +309,7 @@ do_tail (int argc, char *argv[], /* list of files in the guest */ CLEANUP_FREE_STATNS struct guestfs_statns *stat = NULL; if (windows) { - filename = windows_path (g, root, filename, 1 /* readonly */); + filename = windows_path (g, root, argv[i], 1 /* readonly */); if (filename == NULL) return -1; /* windows_path printed an error */ } -- 2.9.3

In case there are no event handlers registered with the handle, get_all_event_callbacks will count 0 elements, trying to malloc a buffer of that size. POSIX says that this can result in either a null pointer, or an unusable pointer. Since we assume a null pointer means failure, then always add a null element at the end, so we do not rely on implementation-defined behaviour of malloc. The output parameter 'len_rtn' already keeps the number of valid items in the returned array, so there are no behaviour changes for callers of get_all_event_callbacks. --- ocaml/guestfs-c.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ocaml/guestfs-c.c b/ocaml/guestfs-c.c index 9042752..f14eee3 100644 --- a/ocaml/guestfs-c.c +++ b/ocaml/guestfs-c.c @@ -311,7 +311,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) } /* Copy them into the return array. */ - r = malloc (sizeof (value *) * (*len_rtn)); + r = malloc (sizeof (value *) * (*len_rtn + 1)); if (r == NULL) caml_raise_out_of_memory (); i = 0; @@ -323,6 +323,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) } root = guestfs_next_private (g, &key); } + r[i] = NULL; return r; } -- 2.9.3

On Friday, 3 March 2017 16:09:33 CET Richard W.M. Jones wrote: This could work for OCaml and Ruby, because NULL as return value is not mishandled, and they throw already errors. In the Python implementation, NULL short-circuits the guestfs_int_py_close flow. I'll change further the implementations, so when there are no handlers no memory is allocated at all. Thanks, -- Pino Toscano

In case there are no event handlers registered with the handle, get_all_event_callbacks will count 0 elements, trying to malloc a buffer of that size. POSIX says that this can result in either a null pointer, or an unusable pointer. Since we assume a null pointer means failure, then always add a null element at the end, so we do not rely on implementation-defined behaviour of malloc. The output parameter 'len_rtn' already keeps the number of valid items in the returned array, so there are no behaviour changes for callers of get_all_event_callbacks. --- ruby/ext/guestfs/handle.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/ruby/ext/guestfs/handle.c b/ruby/ext/guestfs/handle.c index aa10825..0fa230c 100644 --- a/ruby/ext/guestfs/handle.c +++ b/ruby/ext/guestfs/handle.c @@ -385,7 +385,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) } /* Copy them into the return array. */ - r = malloc (sizeof (VALUE *) * (*len_rtn)); + r = malloc (sizeof (VALUE *) * (*len_rtn + 1)); if (r == NULL) rb_raise (rb_eNoMemError, "malloc: %m"); @@ -398,6 +398,7 @@ get_all_event_callbacks (guestfs_h *g, size_t *len_rtn) } root = guestfs_next_private (g, &key); } + r[i] = NULL; return r; } -- 2.9.3