Libguestfs - Sep 2019 - [PATCH v2 5/5] build: remove unused gnulib modules

This is an extended version of:
https://www.redhat.com/archives/libguestfs/2019-September/msg00178.html

It adds a couple of simple code changes, so it makes it possible to
drop more modules.  In addition, more unused modules were dropped.

Pino Toscano (5):
  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

 bootstrap                          | 17 ---------
 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 +++---
 11 files changed, 66 insertions(+), 149 deletions(-)

-- 
2.21.0

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

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

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

---
 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

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

Series looks good, although you might consider squashing 2, 3 and 4
together since they're really part of the same change.

ACK

Thanks,

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