similar to: [PATCH] daemon: Remove use of fixed-size stack buffers.

v1 -> v2: - Fixes as suggested by Pino. Rich.

GCC 7 complains that the fixed size buffers are not large enough (at least in theory) when using ‘-O3 -mtune=broadwell’. --- daemon/devsparts.c | 16 +++++++++++----- 1 file changed, 11 insertions(+), 5 deletions(-) diff --git a/daemon/devsparts.c b/daemon/devsparts.c index 584e7d8b8..82467b92f 100644 --- a/daemon/devsparts.c +++ b/daemon/devsparts.c @@ -125,13 +125,16 @@ foreach_block_device

From: Pino Toscano <ptoscano@redhat.com> If the device name ends with a number, Linux uses partition names of the form <device>p<N>. Handle this case by knocking off the 'p' character. --- daemon/devsparts.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/daemon/devsparts.c b/daemon/devsparts.c index 5862ae2..b764f63 100644 --- a/daemon/devsparts.c +++

Also: * Un-duplicate device detection code by creating a common mapping function. * Add some more comments. --- daemon/devsparts.c | 209 +++++++++++++++++++++++++++++----------------------- 1 files changed, 116 insertions(+), 93 deletions(-) diff --git a/daemon/devsparts.c b/daemon/devsparts.c index 33579ba..0e056a1 100644 --- a/daemon/devsparts.c +++ b/daemon/devsparts.c @@ -29,57 +29,38 @@

The reimplemented APIs are: * list_devices * list_partitions * part_to_dev * part_to_partnum * is_whole_device --- daemon/Makefile.am | 2 + daemon/daemon.h | 3 - daemon/devsparts.c | 257 ---------------------------------------------- daemon/devsparts.ml | 109 ++++++++++++++++++++ daemon/devsparts.mli | 25 +++++ daemon/guestfsd.c | 75

From: Pino Toscano <ptoscano@redhat.com> --- daemon/devsparts.c | 16 ++++++++++++---- 1 file changed, 12 insertions(+), 4 deletions(-) diff --git a/daemon/devsparts.c b/daemon/devsparts.c index b764f63..852ee99 100644 --- a/daemon/devsparts.c +++ b/daemon/devsparts.c @@ -20,6 +20,7 @@ #include <stdio.h> #include <stdlib.h> +#include <stdbool.h> #include

v1 -> v2: - Use intprops macro suggested by danpb. Rich.

This happens with GCC 7.0.1. The errors were all of the form: qemu-speed-test.c: In function 'main': qemu-speed-test.c:153:7: error: this statement may fall through [-Werror=implicit-fallthrough=] usage (EXIT_SUCCESS); ^~~~~~~~~~~~~~~~~~~~ qemu-speed-test.c:155:5: note: here default: ^~~~~~~ --- builder/index-validate.c | 2 +-

GCC 7.0.1 can determine if there is likely to be sufficient space in the output buffer when using sprintf/snprintf, based on the format string. The errors were all either of this form: bindtests.c:717:29: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=] snprintf (strs[i], 16, "%zu", i);

GCC 7.0.1 can determine if there is likely to be sufficient space in the output buffer when using sprintf/snprintf, based on the format string. The errors were all either of this form: bindtests.c:717:29: error: '%zu' directive output may be truncated writing between 1 and 19 bytes into a region of size 16 [-Werror=format-truncation=] snprintf (strs[i], 16, "%zu", i);

These two patches are probably not completely independent, but separating them is a lot of work. With *both* patches applied, all the tests and extra-tests pass. That's no guarantee however that there isn't a mistake, so I don't think this patch is a candidate for the 1.16 branch, until it's had a lot more testing in development. Rich.

If add_string_nodup fails free the passed string instead of leaking it, as that string would have been owned by the stringbuf. Adapt few places to this behaviour. --- daemon/btrfs.c | 4 +--- daemon/devsparts.c | 8 ++++---- daemon/guestfsd.c | 1 + 3 files changed, 6 insertions(+), 7 deletions(-) diff --git a/daemon/btrfs.c b/daemon/btrfs.c index 9b52aa8..d70565a 100644 ---

Declare most of the stringsbuf as CLEANUP_FREE_STRINGSBUF, so they are freed completely on stack unwind: use take_stringsbuf() in return places to take away from the stringsbuf its content, and remove all the manual calls to free_stringslen (no more needed now). This requires to not use free_stringslen anymore on failure in the helper functions of stringsbuf, which now leave the content as-is

Version 1 was here: https://www.redhat.com/archives/libguestfs/2017-June/msg00003.html This patch series reimplements a few more APIs in OCaml, including some very important core APIs like ?list_filesystems? and ?mount?. All the tests pass after this. The selection of APIs that I have moved may look a little random, but in fact they are all APIs consumed by the inspection code (and some more

I think this fixes everything mentioned: - Added the Optgroups module as suggested. - Remove command temporary files. - Replace command ~flags with ?fold_stdout_on_stderr. - Nest _with_mounted function. - Rebase & retest. Rich.

v1 was posted here: https://www.redhat.com/archives/libguestfs/2017-July/msg00098.html This series now depends on two small patches which I posted separately: https://www.redhat.com/archives/libguestfs/2017-July/msg00207.html https://www.redhat.com/archives/libguestfs/2017-July/msg00209.html v1 -> v2: - Previously changes to generator/daemon.ml were made incrementally through the patch

v2 was here: https://www.redhat.com/archives/libguestfs/2017-June/msg00008.html This series gets as far as a working (and faster) reimplementation of ‘guestfs_list_filesystems’. I also have another patch series on top of this one which reimplements the inspection APIs inside the daemon, but that needs a bit more work still, since inspection turns out to be a very large piece of code. Rich.

Previously posted as part of the mega utilities/inspection series here: https://www.redhat.com/archives/libguestfs/2017-June/msg00232.html What I've done is to extract just the parts related to rewriting daemon APIs in OCaml, rebase them on top of the current master, fix a few things, and recompile and test everything. Rich.

I ran the Coverity static analyzer[1] on libguestfs, and fixed many errors as a result. Coverity found some errors in gnulib, but it doesn't seem to be worth following those up since the version of gnulib we are using is so old. There are a couple more errors (possibly 1 false-positive) which I'm going to send in a separate email. BTW all the errors found by Coverity were in the daemon

match will expand to guestfs___match, rename the local variable to avoid clash. Signed-off-by: Olaf Hering <olaf at aepfle.de> diff --git a/src/inspect-fs-unix.c b/src/inspect-fs-unix.c index 06ff96d..c30ad5a 100644 --- a/src/inspect-fs-unix.c +++ b/src/inspect-fs-unix.c @@ -1128,14 +1128,14 @@ map_md_devices(guestfs_h *g, Hash_table **map)