This is an extended version of: https://www.redhat.com/archives/libguestfs/2019-September/msg00288.html Apparently I forgot it on my tree, so I'm posting that series again, adding an extra cleanup more due to the v2v/common splits. Pino Toscano (6): tests: switch away from xgetcwd daemon: move read_whole_file to common utils daemon: switch from read_file to read_whole_file daemon: remove unused include build: remove unused gnulib modules build: ignore unused submodules Makefile.am | 4 --- bootstrap | 17 --------- configure.ac | 2 -- daemon/9p.c | 58 +----------------------------- daemon/Makefile.am | 4 --- daemon/cpio.c | 2 -- daemon/daemon.h | 1 + daemon/ntfsclone.c | 4 +-- daemon/tar.c | 4 +-- daemon/utils.c | 58 ++++++++++++++++++++++++++++++ lib/Makefile.am | 5 --- m4/.gitignore | 52 --------------------------- tests/c-api/test-add-libvirt-dom.c | 10 +++--- 13 files changed, 66 insertions(+), 155 deletions(-) -- 2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 1/6] tests: switch away from xgetcwd
xgetcwd is used only in a test, so there is no need to pull a gnulib
module just for it.
Switch to use getcwd directly with a fixed buffer: the tests would have
failed with paths longer than 992 characters, as the libvirt_uri would
have been truncated. Since there were no reports of issues, we can
assume that the current working directory will fit in 1024 characters;
adapt the size of libvirt_uri accordingly.
---
tests/c-api/test-add-libvirt-dom.c | 10 ++++------
1 file changed, 4 insertions(+), 6 deletions(-)
diff --git a/tests/c-api/test-add-libvirt-dom.c
b/tests/c-api/test-add-libvirt-dom.c
index 10224d102..7c7c1bd26 100644
--- a/tests/c-api/test-add-libvirt-dom.c
+++ b/tests/c-api/test-add-libvirt-dom.c
@@ -28,8 +28,6 @@
#include <libvirt/libvirt.h>
#include <libvirt/virterror.h>
-#include "xgetcwd.h"
-
#include "guestfs.h"
#include "guestfs-utils.h"
@@ -76,11 +74,12 @@ main (int argc, char *argv[])
virErrorPtr err;
int r;
char *backend;
- char *cwd;
+ char cwd[1024];
FILE *fp;
- char libvirt_uri[1024];
+ char libvirt_uri[sizeof cwd + 64];
- cwd = xgetcwd ();
+ if (getcwd (cwd, sizeof cwd) == NULL)
+ error (EXIT_FAILURE, errno, "getcwd");
/* Create the guestfs handle. */
g = guestfs_create ();
@@ -147,7 +146,6 @@ main (int argc, char *argv[])
virDomainFree (dom);
virConnectClose (conn);
- free (cwd);
unlink ("test-add-libvirt-dom.xml");
unlink ("test-add-libvirt-dom-1.img");
--
2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 2/6] daemon: move read_whole_file to common utils
Move the read_whole_file function to the common utilities of the daemon,
so other parts can use it. For this purpose, add an out parameter to
get the amount of bytes read.
Except from the parameter addition, this should be just refactoring.
---
daemon/9p.c | 58 +------------------------------------------------
daemon/daemon.h | 1 +
daemon/utils.c | 58 +++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 60 insertions(+), 57 deletions(-)
diff --git a/daemon/9p.c b/daemon/9p.c
index 55644249d..ce73e2ba0 100644
--- a/daemon/9p.c
+++ b/daemon/9p.c
@@ -34,8 +34,6 @@
#define BUS_PATH "/sys/bus/virtio/drivers/9pnet_virtio"
-static char *read_whole_file (const char *filename);
-
/* https://bugzilla.redhat.com/show_bug.cgi?id=714981#c1 */
char **
do_list_9p (void)
@@ -82,7 +80,7 @@ do_list_9p (void)
* the mount tag length to be unlimited (or up to 65536 bytes).
* See: linux/include/linux/virtio_9p.h
*/
- CLEANUP_FREE char *mount_tag = read_whole_file (mount_tag_path);
+ CLEANUP_FREE char *mount_tag = read_whole_file (mount_tag_path, NULL);
if (mount_tag == 0)
continue;
@@ -117,60 +115,6 @@ do_list_9p (void)
return take_stringsbuf (&r);
}
-/* Read whole file into dynamically allocated array. If there is an
- * error, DON'T call reply_with_perror, just return NULL. Returns a
- * \0-terminated string.
- */
-static char *
-read_whole_file (const char *filename)
-{
- char *r = NULL;
- size_t alloc = 0, size = 0;
- int fd;
-
- fd = open (filename, O_RDONLY|O_CLOEXEC);
- if (fd == -1) {
- perror (filename);
- return NULL;
- }
-
- while (1) {
- alloc += 256;
- char *r2 = realloc (r, alloc);
- if (r2 == NULL) {
- perror ("realloc");
- free (r);
- close (fd);
- return NULL;
- }
- r = r2;
-
- /* The '- 1' in the size calculation ensures there is space below
- * to add \0 to the end of the input.
- */
- ssize_t n = read (fd, r + size, alloc - size - 1);
- if (n == -1) {
- fprintf (stderr, "read: %s: %m\n", filename);
- free (r);
- close (fd);
- return NULL;
- }
- if (n == 0)
- break;
- size += n;
- }
-
- if (close (fd) == -1) {
- fprintf (stderr, "close: %s: %m\n", filename);
- free (r);
- return NULL;
- }
-
- r[size] = '\0';
-
- return r;
-}
-
/* Takes optional arguments, consult optargs_bitmask. */
int
do_mount_9p (const char *mount_tag, const char *mountpoint, const char
*options)
diff --git a/daemon/daemon.h b/daemon/daemon.h
index 66bfdc49e..170fb2537 100644
--- a/daemon/daemon.h
+++ b/daemon/daemon.h
@@ -84,6 +84,7 @@ extern int random_name (char *template);
extern char *get_random_uuid (void);
extern char *make_exclude_from_file (const char *function, char *const
*excludes);
extern int asprintf_nowarn (char **strp, const char *fmt, ...);
+extern char *read_whole_file (const char *filename, size_t *size_r);
/* mountable functions (in utils.c) */
extern char *mountable_to_string (const mountable_t *mountable);
diff --git a/daemon/utils.c b/daemon/utils.c
index c3f88bcab..118965225 100644
--- a/daemon/utils.c
+++ b/daemon/utils.c
@@ -827,3 +827,61 @@ cleanup_free_mountable (mountable_t *mountable)
free (mountable->volume);
}
}
+
+/**
+ * Read whole file into dynamically allocated array. If there is an
+ * error, DON'T call reply_with_perror, just return NULL. Returns a
+ * C<\0>-terminated string. C<size_r> can be specified to get the
+ * size of the returned data.
+ */
+char *
+read_whole_file (const char *filename, size_t *size_r)
+{
+ char *r = NULL;
+ size_t alloc = 0, size = 0;
+ int fd;
+
+ fd = open (filename, O_RDONLY|O_CLOEXEC);
+ if (fd == -1) {
+ perror (filename);
+ return NULL;
+ }
+
+ while (1) {
+ alloc += 256;
+ char *r2 = realloc (r, alloc);
+ if (r2 == NULL) {
+ perror ("realloc");
+ free (r);
+ close (fd);
+ return NULL;
+ }
+ r = r2;
+
+ /* The '- 1' in the size calculation ensures there is space below
+ * to add \0 to the end of the input.
+ */
+ ssize_t n = read (fd, r + size, alloc - size - 1);
+ if (n == -1) {
+ fprintf (stderr, "read: %s: %m\n", filename);
+ free (r);
+ close (fd);
+ return NULL;
+ }
+ if (n == 0)
+ break;
+ size += n;
+ }
+
+ if (close (fd) == -1) {
+ fprintf (stderr, "close: %s: %m\n", filename);
+ free (r);
+ return NULL;
+ }
+
+ r[size] = '\0';
+ if (size_r != NULL)
+ *size_r = size;
+
+ return r;
+}
--
2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 3/6] daemon: switch from read_file to read_whole_file
Since we have already an helper to read a file, use it instead of the
gnulib function.
---
daemon/ntfsclone.c | 4 +---
daemon/tar.c | 4 +---
2 files changed, 2 insertions(+), 6 deletions(-)
diff --git a/daemon/ntfsclone.c b/daemon/ntfsclone.c
index 2e481eb94..3c0d10fc5 100644
--- a/daemon/ntfsclone.c
+++ b/daemon/ntfsclone.c
@@ -26,8 +26,6 @@
#include <errno.h>
#include <error.h>
-#include "read-file.h"
-
#include "daemon.h"
#include "actions.h"
#include "optgroups.h"
@@ -39,7 +37,7 @@ read_error_file (char *error_file)
size_t len;
char *str;
- str = read_file (error_file, &len);
+ str = read_whole_file (error_file, &len);
if (str == NULL) {
str = strdup ("(no error)");
if (str == NULL)
diff --git a/daemon/tar.c b/daemon/tar.c
index 300e99448..daad75a47 100644
--- a/daemon/tar.c
+++ b/daemon/tar.c
@@ -28,8 +28,6 @@
#include <sys/types.h>
#include <sys/stat.h>
-#include "read-file.h"
-
#include "guestfs_protocol.h"
#include "daemon.h"
#include "actions.h"
@@ -107,7 +105,7 @@ read_error_file (char *error_file)
size_t len;
char *str;
- str = read_file (error_file, &len);
+ str = read_whole_file (error_file, &len);
if (str == NULL) {
str = strdup ("(no error)");
if (str == NULL)
--
2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 4/6] daemon: remove unused include
--- daemon/cpio.c | 2 -- 1 file changed, 2 deletions(-) diff --git a/daemon/cpio.c b/daemon/cpio.c index 09491ac9c..c8456af2a 100644 --- a/daemon/cpio.c +++ b/daemon/cpio.c @@ -26,8 +26,6 @@ #include <sys/types.h> #include <sys/stat.h> -#include "read-file.h" - #include "guestfs_protocol.h" #include "daemon.h" #include "actions.h" -- 2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 5/6] build: remove unused gnulib modules
Remove gnulib modules that provide stuff clearly not used within libguestfs (library, daemon, and C tools). Among directly and indirectly modules used previous (and now no more), they are: cycle-check d-ino dev-ino dup3 dup3-tests fcntl-safer fcntl-safer-tests fdopendir fdopendir-tests filevercmp filevercmp-tests ftell ftell-tests ftello ftello-tests fts getaddrinfo getaddrinfo-tests getcwd getcwd-tests gnu-make hostent i-ring i-ring-tests iconv iconv-tests inet_ntop inet_ntop-tests isatty isatty-tests openat-safer openat-safer-tests opendirat ptsname_r ptsname_r-tests read-file read-file-tests rewinddir servent ttyname_r ttyname_r-tests xgetcwd Some of the removed modules are still used pulled indirectly as dependency of other modules. There should be no behaviour change on recent Linux distros, although older distros were not tested (adding a module back is easy, anyway). Remove accordingly unused automake variables, and ignored files. --- bootstrap | 17 --------------- daemon/Makefile.am | 4 ---- lib/Makefile.am | 5 ----- m4/.gitignore | 52 ---------------------------------------------- 4 files changed, 78 deletions(-) diff --git a/bootstrap b/bootstrap index aaf038d98..9e5a8b72d 100755 --- a/bootstrap +++ b/bootstrap @@ -104,34 +104,26 @@ gnulib_tool=$GNULIB_SRCDIR/gnulib-tool modules=' accept4 -areadlink areadlinkat -arpa_inet base64 byteswap c-ctype cloexec closeout connect -dup3 error -filevercmp fstatat -fts full-read full-write futimens -getaddrinfo getline getprogname gitlog-to-changelog glob -gnu-make gnumakefile hash hash-pjw human -iconv ignore-value intprops lock @@ -141,24 +133,18 @@ memmem mkdtemp mkstemps netdb -netinet_in nonblocking -openat perror pipe2 pread -ptsname_r -read-file readlink select setenv sleep socket -stat-time strchrnul strerror strndup -symlinkat sys_select sys_types sys_wait @@ -166,9 +152,6 @@ tls vasprintf vc-list-files warnings -xalloc -xalloc-die -xgetcwd xstrtol xstrtoll xvasprintf diff --git a/daemon/Makefile.am b/daemon/Makefile.am index 49dbf1998..7c71920c4 100644 --- a/daemon/Makefile.am +++ b/daemon/Makefile.am @@ -225,13 +225,9 @@ guestfsd_LDADD = \ $(HIVEX_LIBS) \ $(SD_JOURNAL_LIBS) \ $(top_builddir)/gnulib/lib/.libs/libgnu.a \ - $(GETADDRINFO_LIB) \ - $(HOSTENT_LIB) \ - $(INET_NTOP_LIB) \ $(LIBSOCKET) \ $(LIB_CLOCK_GETTIME) \ $(LIBINTL) \ - $(SERVENT_LIB) \ $(PCRE_LIBS) \ $(TSK_LIBS) \ $(RPC_LIBS) \ diff --git a/lib/Makefile.am b/lib/Makefile.am index 95b5edb4e..e5df96fe2 100644 --- a/lib/Makefile.am +++ b/lib/Makefile.am @@ -154,15 +154,10 @@ libguestfs_la_LIBADD = \ $(SELINUX_LIBS) \ $(JANSSON_LIBS) \ ../gnulib/lib/libgnu.la \ - $(GETADDRINFO_LIB) \ - $(HOSTENT_LIB) \ - $(INET_NTOP_LIB) \ $(LIBSOCKET) \ $(LIB_CLOCK_GETTIME) \ $(LTLIBINTL) \ $(LTLIBTHREAD) \ - $(LTLIBICONV) \ - $(SERVENT_LIB) \ $(RPC_LIBS) # Force libtool to name the library 'libguestfs.so.0.$(MAX_PROC_NR).0'. diff --git a/m4/.gitignore b/m4/.gitignore index 16ae9c415..c41044f84 100644 --- a/m4/.gitignore +++ b/m4/.gitignore @@ -2,51 +2,38 @@ /absolute-header.m4 /accept4.m4 /alloca.m4 -/argmatch.m4 /arpa_inet_h.m4 /asm-underscore.m4 /base64.m4 /btowc.m4 /builtin-expect.m4 /byteswap.m4 -/canonicalize-lgpl.m4 /chdir-long.m4 -/chown.m4 /clock_time.m4 -/cloexec.m4 /closedir.m4 /close.m4 /codeset.m4 /ctype.m4 -/cycle-check.m4 -/d-ino.m4 /dirent_h.m4 /dirfd.m4 /dirname.m4 -/dos.m4 /double-slash-root.m4 /d-type.m4 /dup2.m4 -/dup3.m4 /dup.m4 /eealloc.m4 /environ.m4 /errno_h.m4 /error.m4 -/exitfail.m4 /exponentd.m4 /extensions.m4 /extern-inline.m4 /fatal-signal.m4 /fchdir.m4 -/fclose.m4 /fcntl_h.m4 /fcntl.m4 /fcntl-o.m4 -/fcntl-safer.m4 -/fdopendir.m4 /fdopen.m4 -/fflush.m4 /filenamecat.m4 /flexmember.m4 /float_h.m4 @@ -54,23 +41,13 @@ /fnmatch.m4 /fpending.m4 /fpieee.m4 -/fpurge.m4 -/freading.m4 -/fseeko.m4 /fstatat.m4 /fstat.m4 -/ftell.m4 -/ftello.m4 /ftruncate.m4 -/fts.m4 /futimens.m4 -/getaddrinfo.m4 -/getcwd-abort-bug.m4 /getcwd.m4 -/getcwd-path-max.m4 /getdelim.m4 /getdtablesize.m4 -/getgroups.m4 /getline.m4 /getlogin.m4 /getlogin_r.m4 @@ -79,20 +56,14 @@ /getprogname.m4 /gettime.m4 /gettimeofday.m4 -/getugroups.m4 /glob_h.m4 /glob.m4 /gnulib-common.m4 /gnulib-comp.m4 /gnulib-tool.m4 -/gnu-make.m4 -/hash.m4 /host-cpu-c-abi.m4 -/hostent.m4 /human.m4 -/iconv.m4 /include_next.m4 -/inet_ntop.m4 /inet_pton.m4 /__inline.m4 /intlmacosx.m4 @@ -103,13 +74,9 @@ /inttypes.m4 /inttypes-pri.m4 /ioctl.m4 -/i-ring.m4 -/isatty.m4 /isblank.m4 -/isc-posix.m4 /langinfo_h.m4 /largefile.m4 -/lchown.m4 /lcmessage.m4 /lib-ld.m4 /lib-link.m4 @@ -125,7 +92,6 @@ /locale-zh.m4 /localtime-buffer.m4 /lock.m4 -/longdouble.m4 /longlong.m4 /lseek.m4 /lstat.m4 @@ -143,7 +109,6 @@ /memmem.m4 /mempcpy.m4 /memrchr.m4 -/mgetgroups.m4 /minmax.m4 /mkdir.m4 /mkdtemp.m4 @@ -175,26 +140,22 @@ /pthread_rwlock_rdlock.m4 /pthread_sigmask.m4 /pthread-thread.m4 -/ptsname_r.m4 /putenv.m4 /quotearg.m4 /quote.m4 /raise.m4 /rawmemchr.m4 /readdir.m4 -/read-file.m4 /readlinkat.m4 /readlink.m4 /read.m4 /realloc.m4 -/rewinddir.m4 /rmdir.m4 /safe-read.m4 /safe-write.m4 /save-cwd.m4 /sched_h.m4 /select.m4 -/servent.m4 /setenv.m4 /setlocale.m4 /sh-filename.m4 @@ -202,7 +163,6 @@ /sig_atomic_t.m4 /signalblocking.m4 /signal_h.m4 -/signed.m4 /size_max.m4 /sleep.m4 /snprintf.m4 @@ -230,11 +190,8 @@ /string_h.m4 /strndup.m4 /strnlen.m4 -/strtoimax.m4 /strtoll.m4 -/strtol.m4 /strtoull.m4 -/strtoul.m4 /strtoumax.m4 /symlinkat.m4 /symlink.m4 @@ -252,16 +209,12 @@ /time_h.m4 /timespec.m4 /tls.m4 -/ttyname_r.m4 -/ulonglong.m4 -/ungetc.m4 /unistd_h.m4 /unistd-safer.m4 /unlinkat.m4 /unlinkdir.m4 /unlink.m4 /usleep.m4 -/utimbuf.m4 /utimecmp.m4 /utime_h.m4 /utime.m4 @@ -276,20 +229,15 @@ /warnings.m4 /warn-on-use.m4 /wchar_h.m4 -/wchar.m4 /wchar_t.m4 /wcrtomb.m4 /wctob.m4 /wctomb.m4 /wctype_h.m4 -/wctype.m4 /wint_t.m4 /write.m4 /xalloc.m4 -/xgetcwd.m4 /xsize.m4 -/xstrndup.m4 -/xstrtol.m4 /xstrtol.m4 /xvasprintf.m4 /yield.m4 -- 2.21.0
Pino Toscano
2019-Nov-27 16:17 UTC
[Libguestfs] [PATCH v3 6/6] build: ignore unused submodules
Do not build the mlv2v, and mllibvirt submodules, as they are not used.
---
Makefile.am | 4 ----
configure.ac | 2 --
2 files changed, 6 deletions(-)
diff --git a/Makefile.am b/Makefile.am
index 28f542765..b1e88a42b 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -164,10 +164,6 @@ SUBDIRS += common/mlvisit
SUBDIRS += common/mlxml
SUBDIRS += common/mltools
SUBDIRS += common/mlcustomize
-SUBDIRS += common/mlv2v
-if HAVE_LIBVIRT
-SUBDIRS += common/mllibvirt
-endif
SUBDIRS += customize
SUBDIRS += builder builder/templates
SUBDIRS += get-kernel
diff --git a/configure.ac b/configure.ac
index 7ec2c9b46..4d043dd02 100644
--- a/configure.ac
+++ b/configure.ac
@@ -235,14 +235,12 @@ AC_CONFIG_FILES([Makefile
common/mlaugeas/Makefile
common/mlcustomize/Makefile
common/mlgettext/Makefile
- common/mllibvirt/Makefile
common/mlpcre/Makefile
common/mlprogress/Makefile
common/mlstdutils/Makefile
common/mlstdutils/guestfs_config.ml
common/mltools/Makefile
common/mlutils/Makefile
- common/mlv2v/Makefile
common/mlvisit/Makefile
common/mlxml/Makefile
common/options/Makefile
--
2.21.0
Richard W.M. Jones
2019-Nov-27 19:06 UTC
Re: [Libguestfs] [PATCH v3 6/6] build: ignore unused submodules
On Wed, Nov 27, 2019 at 05:17:31PM +0100, Pino Toscano wrote:> Do not build the mlv2v, and mllibvirt submodules, as they are not used. > --- > Makefile.am | 4 ---- > configure.ac | 2 -- > 2 files changed, 6 deletions(-) > > diff --git a/Makefile.am b/Makefile.am > index 28f542765..b1e88a42b 100644 > --- a/Makefile.am > +++ b/Makefile.am > @@ -164,10 +164,6 @@ SUBDIRS += common/mlvisit > SUBDIRS += common/mlxml > SUBDIRS += common/mltools > SUBDIRS += common/mlcustomize > -SUBDIRS += common/mlv2v > -if HAVE_LIBVIRT > -SUBDIRS += common/mllibvirt > -endif > SUBDIRS += customize > SUBDIRS += builder builder/templates > SUBDIRS += get-kernel > diff --git a/configure.ac b/configure.ac > index 7ec2c9b46..4d043dd02 100644 > --- a/configure.ac > +++ b/configure.ac > @@ -235,14 +235,12 @@ AC_CONFIG_FILES([Makefile > common/mlaugeas/Makefile > common/mlcustomize/Makefile > common/mlgettext/Makefile > - common/mllibvirt/Makefile > common/mlpcre/Makefile > common/mlprogress/Makefile > common/mlstdutils/Makefile > common/mlstdutils/guestfs_config.ml > common/mltools/Makefile > common/mlutils/Makefile > - common/mlv2v/Makefile > common/mlvisit/Makefile > common/mlxml/Makefile > common/options/MakefileACK series Rich. -- Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones Read my programming and virtualization blog: http://rwmj.wordpress.com virt-top is 'top' for virtual machines. Tiny program with many powerful monitoring features, net stats, disk stats, logging, etc. http://people.redhat.com/~rjones/virt-top