Libguestfs - Sep 2017 - [PATCH v2] 0/6] Various clean ups in lib/

v1 -> v2:

 - Remove the unnecessary calls to guestfs_int_lazy_make_tmpdir
   in the final patch.

Rich.

Not used by any current code, removed.

Last use of the function was seen in
commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2.
---
 lib/guestfs-internal.h |  1 -
 lib/inspect.c          | 13 -------------
 2 files changed, 14 deletions(-)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index 35e0cb8e7..a694be8f5 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -715,7 +715,6 @@ extern int guestfs_int_set_backend (guestfs_h *g, const char
*method);
 /* inspect.c */
 extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename,
const char *basename, uint64_t max_size);
 extern int guestfs_int_parse_unsigned_int (guestfs_h *g, const char *str);
-extern int guestfs_int_parse_unsigned_int_ignore_trailing (guestfs_h *g, const
char *str);
 
 /* dbdump.c */
 typedef int (*guestfs_int_db_dump_callback) (guestfs_h *g, const unsigned char
*key, size_t keylen, const unsigned char *value, size_t valuelen, void *opaque);
diff --git a/lib/inspect.c b/lib/inspect.c
index 501074316..b7c5b6cc7 100644
--- a/lib/inspect.c
+++ b/lib/inspect.c
@@ -118,16 +118,3 @@ guestfs_int_parse_unsigned_int (guestfs_h *g, const char
*str)
   }
   return ret;
 }
-
-/* Like parse_unsigned_int, but ignore trailing stuff. */
-int
-guestfs_int_parse_unsigned_int_ignore_trailing (guestfs_h *g, const char *str)
-{
-  long ret;
-  const int r = xstrtol (str, NULL, 10, &ret, NULL);
-  if (r != LONGINT_OK) {
-    error (g, _("could not parse integer in version number: %s"),
str);
-    return -1;
-  }
-  return ret;
-}
-- 
2.13.2

Just code motion, but this function seems to make more sense in
version.c since that is the main (but not only) user.
---
 lib/guestfs-internal.h |  2 +-
 lib/inspect.c          | 14 --------------
 lib/version.c          | 24 ++++++++++++++++++++++++
 3 files changed, 25 insertions(+), 15 deletions(-)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index a694be8f5..d24435011 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -714,7 +714,6 @@ extern int guestfs_int_set_backend (guestfs_h *g, const char
*method);
 
 /* inspect.c */
 extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename,
const char *basename, uint64_t max_size);
-extern int guestfs_int_parse_unsigned_int (guestfs_h *g, const char *str);
 
 /* dbdump.c */
 typedef int (*guestfs_int_db_dump_callback) (guestfs_h *g, const unsigned char
*key, size_t keylen, const unsigned char *value, size_t valuelen, void *opaque);
@@ -805,6 +804,7 @@ extern int guestfs_int_version_from_x_y_re (guestfs_h *g,
struct version *v, con
 extern int guestfs_int_version_from_x_y_or_x (guestfs_h *g, struct version *v,
const char *str);
 extern bool guestfs_int_version_ge (const struct version *v, int maj, int min,
int mic);
 extern bool guestfs_int_version_cmp_ge (const struct version *a, const struct
version *b);
+extern int guestfs_int_parse_unsigned_int (guestfs_h *g, const char *str);
 #define version_init_null(v) guestfs_int_version_from_values (v, 0, 0, 0)
 #define version_is_null(v) ((v)->v_major == 0 && (v)->v_minor ==
0 && (v)->v_micro == 0)
 
diff --git a/lib/inspect.c b/lib/inspect.c
index b7c5b6cc7..eb53edf32 100644
--- a/lib/inspect.c
+++ b/lib/inspect.c
@@ -33,7 +33,6 @@
 #endif
 
 #include "ignore-value.h"
-#include "xstrtol.h"
 
 #include "guestfs.h"
 #include "guestfs-internal.h"
@@ -105,16 +104,3 @@ guestfs_int_download_to_tmp (guestfs_h *g,
   free (r);
   return NULL;
 }
-
-/* Parse small, unsigned ints, as used in version numbers. */
-int
-guestfs_int_parse_unsigned_int (guestfs_h *g, const char *str)
-{
-  long ret;
-  const int r = xstrtol (str, NULL, 10, &ret, "");
-  if (r != LONGINT_OK) {
-    error (g, _("could not parse integer in version number: %s"),
str);
-    return -1;
-  }
-  return ret;
-}
diff --git a/lib/version.c b/lib/version.c
index 60ffe1e89..cf601ef91 100644
--- a/lib/version.c
+++ b/lib/version.c
@@ -22,10 +22,14 @@
 
 #include <config.h>
 
+#include <stdio.h>
+#include <stdlib.h>
 #include <string.h>
 #include <unistd.h>
+#include <libintl.h>
 
 #include "ignore-value.h"
+#include "xstrtol.h"
 
 #include "guestfs.h"
 #include "guestfs-internal.h"
@@ -144,3 +148,23 @@ version_from_x_y_or_x (guestfs_h *g, struct version *v,
const char *str,
   }
   return 0;
 }
+
+/**
+ * Parse small, unsigned ints, as used in version numbers.
+ *
+ * This will fail with an error if trailing characters are found after
+ * the integer.
+ *
+ * Returns E<ge> C<0> on success, or C<-1> on failure.
+ */
+int
+guestfs_int_parse_unsigned_int (guestfs_h *g, const char *str)
+{
+  long ret;
+  const int r = xstrtol (str, NULL, 10, &ret, "");
+  if (r != LONGINT_OK) {
+    error (g, _("could not parse integer in version number: %s"),
str);
+    return -1;
+  }
+  return ret;
+}
-- 
2.13.2

Move the last remaining function ‘guestfs_int_download_to_tmp’ to
lib/inspect-icon.c (the main, but not only user of this function).
Then remove lib/inspect.c entirely.

This is not quite code motion because I updated the comment for the
function to reflect what it does in reality.
---
 docs/C_SOURCE_FILES |   1 -
 lib/Makefile.am     |   1 -
 lib/inspect-icon.c  |  74 +++++++++++++++++++++++++++++++++++-
 lib/inspect.c       | 106 ----------------------------------------------------
 po/POTFILES         |   1 -
 5 files changed, 73 insertions(+), 110 deletions(-)

diff --git a/docs/C_SOURCE_FILES b/docs/C_SOURCE_FILES
index 386df6d9a..2b947a81c 100644
--- a/docs/C_SOURCE_FILES
+++ b/docs/C_SOURCE_FILES
@@ -313,7 +313,6 @@ lib/handle.c
 lib/info.c
 lib/inspect-apps.c
 lib/inspect-icon.c
-lib/inspect.c
 lib/journal.c
 lib/launch-direct.c
 lib/launch-libvirt.c
diff --git a/lib/Makefile.am b/lib/Makefile.am
index 677180636..a1d8ee70e 100644
--- a/lib/Makefile.am
+++ b/lib/Makefile.am
@@ -93,7 +93,6 @@ libguestfs_la_SOURCES = \
 	guid.c \
 	handle.c \
 	info.c \
-	inspect.c \
 	inspect-apps.c \
 	inspect-icon.c \
 	journal.c \
diff --git a/lib/inspect-icon.c b/lib/inspect-icon.c
index f4f5f0660..c45761041 100644
--- a/lib/inspect-icon.c
+++ b/lib/inspect-icon.c
@@ -20,8 +20,11 @@
 
 #include <stdio.h>
 #include <stdlib.h>
-#include <unistd.h>
+#include <inttypes.h>
 #include <string.h>
+#include <unistd.h>
+#include <fcntl.h>
+#include <libintl.h>
 #include <sys/wait.h>
 
 #include "guestfs.h"
@@ -644,3 +647,72 @@ case_sensitive_path_silently (guestfs_h *g, const char
*path)
 
   return ret;
 }
+
+/**
+ * Download a guest file to a local temporary file.  The file is
+ * cached in the temporary directory using C<basename>, and is not
+ * downloaded again.
+ *
+ * The name of the temporary (downloaded) file is returned.  The
+ * caller must free the pointer, but does I<not> need to delete the
+ * temporary file.  It will be deleted when the handle is closed.
+ *
+ * Refuse to download the guest file if it is larger than C<max_size>.
+ * On this and other errors, C<NULL> is returned.
+ *
+ * XXX Prior to commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2 this
+ * function used different basenames for each inspection root.  After
+ * this commit icons probably won't work properly.  Needs fixing.
+ */
+char *
+guestfs_int_download_to_tmp (guestfs_h *g,
+			     const char *filename,
+			     const char *basename, uint64_t max_size)
+{
+  char *r;
+  int fd;
+  char devfd[32];
+  int64_t size;
+
+  if (asprintf (&r, "%s/%s", g->tmpdir, basename) == -1) {
+    perrorf (g, "asprintf");
+    return NULL;
+  }
+
+  /* Check size of remote file. */
+  size = guestfs_filesize (g, filename);
+  if (size == -1)
+    /* guestfs_filesize failed and has already set error in handle */
+    goto error;
+  if ((uint64_t) size > max_size) {
+    error (g, _("size of %s is unreasonably large (%" PRIi64 "
bytes)"),
+           filename, size);
+    goto error;
+  }
+
+  fd = open (r, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0600);
+  if (fd == -1) {
+    perrorf (g, "open: %s", r);
+    goto error;
+  }
+
+  snprintf (devfd, sizeof devfd, "/dev/fd/%d", fd);
+
+  if (guestfs_download (g, filename, devfd) == -1) {
+    unlink (r);
+    close (fd);
+    goto error;
+  }
+
+  if (close (fd) == -1) {
+    perrorf (g, "close: %s", r);
+    unlink (r);
+    goto error;
+  }
+
+  return r;
+
+ error:
+  free (r);
+  return NULL;
+}
diff --git a/lib/inspect.c b/lib/inspect.c
deleted file mode 100644
index eb53edf32..000000000
--- a/lib/inspect.c
+++ /dev/null
@@ -1,106 +0,0 @@
-/* libguestfs
- * Copyright (C) 2010-2012 Red Hat Inc.
- *
- * This library is free software; you can redistribute it and/or
- * modify it under the terms of the GNU Lesser General Public
- * License as published by the Free Software Foundation; either
- * version 2 of the License, or (at your option) any later version.
- *
- * This library is distributed in the hope that it will be useful,
- * but WITHOUT ANY WARRANTY; without even the implied warranty of
- * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
- * Lesser General Public License for more details.
- *
- * You should have received a copy of the GNU Lesser General Public
- * License along with this library; if not, write to the Free Software
- * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
- */
-
-#include <config.h>
-
-#include <stdio.h>
-#include <stdlib.h>
-#include <inttypes.h>
-#include <unistd.h>
-#include <fcntl.h>
-#include <string.h>
-#include <sys/stat.h>
-#include <libintl.h>
-#include <assert.h>
-
-#ifdef HAVE_ENDIAN_H
-#include <endian.h>
-#endif
-
-#include "ignore-value.h"
-
-#include "guestfs.h"
-#include "guestfs-internal.h"
-#include "guestfs-internal-actions.h"
-
-/**
- * Download a guest file to a local temporary file.  The file is
- * cached in the temporary directory, and is not downloaded again.
- *
- * The name of the temporary (downloaded) file is returned.  The
- * caller must free the pointer, but does I<not> need to delete the
- * temporary file.  It will be deleted when the handle is closed.
- *
- * Refuse to download the guest file if it is larger than C<max_size>.
- * On this and other errors, C<NULL> is returned.
- *
- * There is actually one cache per C<struct inspect_fs *> in order to
- * handle the case of multiple roots.
- */
-char *
-guestfs_int_download_to_tmp (guestfs_h *g,
-			     const char *filename,
-			     const char *basename, uint64_t max_size)
-{
-  char *r;
-  int fd;
-  char devfd[32];
-  int64_t size;
-
-  if (asprintf (&r, "%s/%s", g->tmpdir, basename) == -1) {
-    perrorf (g, "asprintf");
-    return NULL;
-  }
-
-  /* Check size of remote file. */
-  size = guestfs_filesize (g, filename);
-  if (size == -1)
-    /* guestfs_filesize failed and has already set error in handle */
-    goto error;
-  if ((uint64_t) size > max_size) {
-    error (g, _("size of %s is unreasonably large (%" PRIi64 "
bytes)"),
-           filename, size);
-    goto error;
-  }
-
-  fd = open (r, O_WRONLY|O_CREAT|O_TRUNC|O_NOCTTY|O_CLOEXEC, 0600);
-  if (fd == -1) {
-    perrorf (g, "open: %s", r);
-    goto error;
-  }
-
-  snprintf (devfd, sizeof devfd, "/dev/fd/%d", fd);
-
-  if (guestfs_download (g, filename, devfd) == -1) {
-    unlink (r);
-    close (fd);
-    goto error;
-  }
-
-  if (close (fd) == -1) {
-    perrorf (g, "close: %s", r);
-    unlink (r);
-    goto error;
-  }
-
-  return r;
-
- error:
-  free (r);
-  return NULL;
-}
diff --git a/po/POTFILES b/po/POTFILES
index 182c80bbd..7d58151b3 100644
--- a/po/POTFILES
+++ b/po/POTFILES
@@ -376,7 +376,6 @@ lib/handle.c
 lib/info.c
 lib/inspect-apps.c
 lib/inspect-icon.c
-lib/inspect.c
 lib/journal.c
 lib/launch-direct.c
 lib/launch-libvirt.c
-- 
2.13.2

In the function ‘guestfs_int_make_temp_path’, allow an optional
extension to be specified.  For example:

  r = guestfs_int_make_temp_path (g, "ls", "txt");

will create paths like ‘<tmpdir>/ls123.txt’.

NULL can also be passed for the extension, which means no extension.
---
 lib/drives.c           | 2 +-
 lib/file.c             | 8 ++++----
 lib/guestfs-internal.h | 2 +-
 lib/journal.c          | 2 +-
 lib/tmpdirs.c          | 8 ++++++--
 lib/tsk.c              | 4 ++--
 lib/yara.c             | 2 +-
 7 files changed, 16 insertions(+), 12 deletions(-)

diff --git a/lib/drives.c b/lib/drives.c
index ec8bdbbe4..117c8bf85 100644
--- a/lib/drives.c
+++ b/lib/drives.c
@@ -421,7 +421,7 @@ create_drive_dev_null (guestfs_h *g,
     data->format = "raw";
   }
 
-  tmpfile = guestfs_int_make_temp_path (g, "devnull");
+  tmpfile = guestfs_int_make_temp_path (g, "devnull",
"img");
   if (tmpfile == NULL)
     return NULL;
 
diff --git a/lib/file.c b/lib/file.c
index 73c983c2b..fa3189a71 100644
--- a/lib/file.c
+++ b/lib/file.c
@@ -87,7 +87,7 @@ guestfs_impl_read_file (guestfs_h *g, const char *path, size_t
*size_r)
   char *ret = NULL;
   struct stat statbuf;
 
-  tmpfile = guestfs_int_make_temp_path (g, "cat");
+  tmpfile = guestfs_int_make_temp_path (g, "cat", NULL);
   if (tmpfile == NULL)
     goto err;
 
@@ -213,7 +213,7 @@ guestfs_impl_find (guestfs_h *g, const char *directory)
   char **ret = NULL;
   size_t i, count, size;
 
-  tmpfile = guestfs_int_make_temp_path (g, "find");
+  tmpfile = guestfs_int_make_temp_path (g, "find", "txt");
   if (tmpfile == NULL)
     goto err;
 
@@ -317,7 +317,7 @@ write_or_append (guestfs_h *g, const char *path,
       (g, path, content, size);
 
   /* Write the content out to a temporary file. */
-  tmpfile = guestfs_int_make_temp_path (g, "write");
+  tmpfile = guestfs_int_make_temp_path (g, "write", NULL);
   if (tmpfile == NULL)
     goto err;
 
@@ -513,7 +513,7 @@ guestfs_impl_ls (guestfs_h *g, const char *directory)
   char **ret = NULL;
   size_t i, count, size;
 
-  tmpfile = guestfs_int_make_temp_path (g, "ls");
+  tmpfile = guestfs_int_make_temp_path (g, "ls", "txt");
   if (tmpfile == NULL)
     goto err;
 
diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index d24435011..f3379da7d 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -661,7 +661,7 @@ extern int guestfs_int_set_env_tmpdir (guestfs_h *g, const
char *envname, const
 extern int guestfs_int_set_env_runtimedir (guestfs_h *g, const char *envname,
const char *runtimedir);
 extern int guestfs_int_lazy_make_tmpdir (guestfs_h *g);
 extern int guestfs_int_lazy_make_sockdir (guestfs_h *g);
-extern char *guestfs_int_make_temp_path (guestfs_h *g, const char *name);
+extern char *guestfs_int_make_temp_path (guestfs_h *g, const char *name, const
char *extension);
 extern char *guestfs_int_lazy_make_supermin_appliance_dir (guestfs_h *g);
 extern void guestfs_int_remove_tmpdir (guestfs_h *g);
 extern void guestfs_int_remove_sockdir (guestfs_h *g);
diff --git a/lib/journal.c b/lib/journal.c
index f36e66117..75a16c8bc 100644
--- a/lib/journal.c
+++ b/lib/journal.c
@@ -66,7 +66,7 @@ guestfs_impl_journal_get (guestfs_h *g)
   size_t i, j, size;
   uint64_t len;
 
-  tmpfile = guestfs_int_make_temp_path (g, "journal");
+  tmpfile = guestfs_int_make_temp_path (g, "journal", NULL);
   if (tmpfile == NULL)
     goto err;
 
diff --git a/lib/tmpdirs.c b/lib/tmpdirs.c
index 344475d1b..605bf90fb 100644
--- a/lib/tmpdirs.c
+++ b/lib/tmpdirs.c
@@ -228,7 +228,8 @@ guestfs_int_lazy_make_sockdir (guestfs_h *g)
  * Returns a unique path or NULL on error.
  */
 char *
-guestfs_int_make_temp_path (guestfs_h *g, const char *name)
+guestfs_int_make_temp_path (guestfs_h *g,
+                            const char *name, const char *extension)
 {
   int ret = 0;
 
@@ -236,7 +237,10 @@ guestfs_int_make_temp_path (guestfs_h *g, const char *name)
   if (ret < 0)
     return NULL;
 
-  return safe_asprintf (g, "%s/%s%d", g->tmpdir, name,
++g->unique);
+  return safe_asprintf (g, "%s/%s%d%s%s",
+                        g->tmpdir, name, ++g->unique,
+                        extension ? "." : "",
+                        extension ? extension : "");
 }
 
 /**
diff --git a/lib/tsk.c b/lib/tsk.c
index da9ca8dad..09e514bd2 100644
--- a/lib/tsk.c
+++ b/lib/tsk.c
@@ -43,7 +43,7 @@ guestfs_impl_filesystem_walk (guestfs_h *g, const char
*mountable)
   int ret = 0;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;
 
-  tmpfile = guestfs_int_make_temp_path (g, "filesystem_walk");
+  tmpfile = guestfs_int_make_temp_path (g, "filesystem_walk", NULL);
   if (tmpfile == NULL)
     return NULL;
 
@@ -60,7 +60,7 @@ guestfs_impl_find_inode (guestfs_h *g, const char *mountable,
int64_t inode)
   int ret = 0;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;
 
-  tmpfile = guestfs_int_make_temp_path (g, "find_inode");
+  tmpfile = guestfs_int_make_temp_path (g, "find_inode", NULL);
   if (tmpfile == NULL)
     return NULL;
 
diff --git a/lib/yara.c b/lib/yara.c
index 7e4683101..edced3be8 100644
--- a/lib/yara.c
+++ b/lib/yara.c
@@ -43,7 +43,7 @@ guestfs_impl_yara_scan (guestfs_h *g, const char *path)
   int r;
   CLEANUP_UNLINK_FREE char *tmpfile = NULL;
 
-  tmpfile = guestfs_int_make_temp_path (g, "yara_scan");
+  tmpfile = guestfs_int_make_temp_path (g, "yara_scan", NULL);
   if (tmpfile == NULL)
     return NULL;
 
-- 
2.13.2

Since commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2,
‘guestfs_int_download_to_tmp’ was buggy because it did not deal with
the case where a guest had multiple roots.  It cached the downloaded
file under a single name which was not distinguished by which root we
were looking at.  As a result, if you inspected (eg.) the icon on such
a guest, then in some circumstances the same icon could be returned
for all roots (incorrectly).

This changes the function in a few ways:

 - Don't cache downloaded files.  It wasn't a useful feature of the
   function in most scenarios.  Instead a new name is generated for
   every call.

 - Use guestfs_int_make_temp_path.

 - Allow an extension to be specified.
---
 lib/guestfs-internal.h |  2 +-
 lib/inspect-apps.c     | 12 +++++-------
 lib/inspect-icon.c     | 36 +++++++++++++++---------------------
 3 files changed, 21 insertions(+), 29 deletions(-)

diff --git a/lib/guestfs-internal.h b/lib/guestfs-internal.h
index f3379da7d..66e03c3b8 100644
--- a/lib/guestfs-internal.h
+++ b/lib/guestfs-internal.h
@@ -713,7 +713,7 @@ extern int guestfs_int_set_backend (guestfs_h *g, const char
*method);
   } while (0)
 
 /* inspect.c */
-extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename,
const char *basename, uint64_t max_size);
+extern char *guestfs_int_download_to_tmp (guestfs_h *g, const char *filename,
const char *extension, uint64_t max_size);
 
 /* dbdump.c */
 typedef int (*guestfs_int_db_dump_callback) (guestfs_h *g, const unsigned char
*key, size_t keylen, const unsigned char *value, size_t valuelen, void *opaque);
diff --git a/lib/inspect-apps.c b/lib/inspect-apps.c
index b721595da..f0cf16b38 100644
--- a/lib/inspect-apps.c
+++ b/lib/inspect-apps.c
@@ -370,14 +370,12 @@ list_applications_rpm (guestfs_h *g, const char *root)
   struct guestfs_application2_list *apps = NULL;
   struct read_package_data data;
 
-  Name = guestfs_int_download_to_tmp (g,
-				      "/var/lib/rpm/Name", "rpm_Name",
+  Name = guestfs_int_download_to_tmp (g, "/var/lib/rpm/Name", NULL,
 				      MAX_PKG_DB_SIZE);
   if (Name == NULL)
     goto error;
 
-  Packages = guestfs_int_download_to_tmp (g,
-					  "/var/lib/rpm/Packages", "rpm_Packages",
+  Packages = guestfs_int_download_to_tmp (g, "/var/lib/rpm/Packages",
NULL,
 					  MAX_PKG_DB_SIZE);
   if (Packages == NULL)
     goto error;
@@ -429,7 +427,7 @@ list_applications_deb (guestfs_h *g, const char *root)
   char **continuation_field = NULL;
   size_t continuation_field_len = 0;
 
-  status = guestfs_int_download_to_tmp (g, "/var/lib/dpkg/status",
"status",
+  status = guestfs_int_download_to_tmp (g, "/var/lib/dpkg/status",
NULL,
 					MAX_PKG_DB_SIZE);
   if (status == NULL)
     return NULL;
@@ -605,7 +603,7 @@ list_applications_pacman (guestfs_h *g, const char *root)
     fname = safe_malloc (g, strlen (curr->name) + path_len + 1);
     sprintf (fname, "/var/lib/pacman/local/%s/desc", curr->name);
     free (desc_file);
-    desc_file = guestfs_int_download_to_tmp (g, fname, curr->name, 8192);
+    desc_file = guestfs_int_download_to_tmp (g, fname, NULL, 8192);
 
     /* The desc files are small (4K). If the desc file does not exist or is
      * larger than the 8K limit we've used, the database is probably
corrupted,
@@ -714,7 +712,7 @@ list_applications_apk (guestfs_h *g, const char *root)
     *url = NULL, *description = NULL;
 
   installed = guestfs_int_download_to_tmp (g,
"/lib/apk/db/installed",
-                                           "installed",
MAX_PKG_DB_SIZE);
+                                           NULL, MAX_PKG_DB_SIZE);
   if (installed == NULL)
     return NULL;
 
diff --git a/lib/inspect-icon.c b/lib/inspect-icon.c
index c45761041..3e44b23eb 100644
--- a/lib/inspect-icon.c
+++ b/lib/inspect-icon.c
@@ -234,7 +234,7 @@ get_png (guestfs_h *g, const char *filename, size_t *size_r,
size_t max_size)
   if (max_size == 0)
     max_size = 4 * w * h;
 
-  local = guestfs_int_download_to_tmp (g, real, "icon", max_size);
+  local = guestfs_int_download_to_tmp (g, real, "png", max_size);
   if (!local)
     return NOT_FOUND;
 
@@ -385,7 +385,7 @@ icon_cirros (guestfs_h *g, size_t *size_r)
   if (!STRPREFIX (type, "ASCII text"))
     return NOT_FOUND;
 
-  local = guestfs_int_download_to_tmp (g, CIRROS_LOGO, "icon", 1024);
+  local = guestfs_int_download_to_tmp (g, CIRROS_LOGO, "png", 1024);
   if (!local)
     return NOT_FOUND;
 
@@ -469,8 +469,7 @@ icon_windows_xp (guestfs_h *g, const char *systemroot,
size_t *size_r)
   if (r == 0)
     return NOT_FOUND;
 
-  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
-						     "explorer.exe",
+  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
"exe",
 						     MAX_WINDOWS_EXPLORER_SIZE);
   if (filename_downloaded == NULL)
     return NOT_FOUND;
@@ -538,8 +537,7 @@ icon_windows_7 (guestfs_h *g, const char *systemroot, size_t
*size_r)
   if (win7_explorer[i] == NULL)
     return NOT_FOUND;
 
-  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
-						     "explorer.exe",
+  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
"exe",
 						     MAX_WINDOWS_EXPLORER_SIZE);
   if (filename_downloaded == NULL)
     return NOT_FOUND;
@@ -592,8 +590,8 @@ icon_windows_8 (guestfs_h *g, size_t *size_r)
   if (r == 0)
     return NOT_FOUND;
 
-  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
-						     "wlive48x48.png", 8192);
+  filename_downloaded = guestfs_int_download_to_tmp (g, filename_case,
"png",
+						     8192);
   if (filename_downloaded == NULL)
     return NOT_FOUND;
 
@@ -649,35 +647,31 @@ case_sensitive_path_silently (guestfs_h *g, const char
*path)
 }
 
 /**
- * Download a guest file to a local temporary file.  The file is
- * cached in the temporary directory using C<basename>, and is not
- * downloaded again.
+ * Download a guest file to a local temporary file.
  *
  * The name of the temporary (downloaded) file is returned.  The
  * caller must free the pointer, but does I<not> need to delete the
  * temporary file.  It will be deleted when the handle is closed.
  *
+ * The name of the temporary file is randomly generated, but an
+ * extension can be specified using C<extension> (or pass C<NULL>
for none).
+ *
  * Refuse to download the guest file if it is larger than C<max_size>.
  * On this and other errors, C<NULL> is returned.
- *
- * XXX Prior to commit 65cfecb0f5344ec92d3f0d3c2ec0538b6b2726e2 this
- * function used different basenames for each inspection root.  After
- * this commit icons probably won't work properly.  Needs fixing.
  */
 char *
-guestfs_int_download_to_tmp (guestfs_h *g,
-			     const char *filename,
-			     const char *basename, uint64_t max_size)
+guestfs_int_download_to_tmp (guestfs_h *g, const char *filename,
+                             const char *extension,
+                             uint64_t max_size)
 {
   char *r;
   int fd;
   char devfd[32];
   int64_t size;
 
-  if (asprintf (&r, "%s/%s", g->tmpdir, basename) == -1) {
-    perrorf (g, "asprintf");
+  r = guestfs_int_make_temp_path (g, "download", extension);
+  if (r == NULL)
     return NULL;
-  }
 
   /* Check size of remote file. */
   size = guestfs_filesize (g, filename);
-- 
2.13.2

There were various places in the library where we open coded making
temporary filenames.  This uses the utility function instead.
---
 lib/appliance-uefi.c |  4 +++-
 lib/command.c        |  6 ++----
 lib/drives.c         |  4 ++--
 lib/inspect-icon.c   | 12 +++++++++---
 lib/launch-direct.c  |  5 ++---
 lib/launch-libvirt.c |  5 ++---
 lib/launch-uml.c     |  5 ++---
 7 files changed, 22 insertions(+), 19 deletions(-)

diff --git a/lib/appliance-uefi.c b/lib/appliance-uefi.c
index 986989e67..38dd2ef6d 100644
--- a/lib/appliance-uefi.c
+++ b/lib/appliance-uefi.c
@@ -74,7 +74,9 @@ guestfs_int_get_uefi (guestfs_h *g, char **code, char **vars,
int *flags)
        * into the address space read-only as that triggers a different
        * path inside UEFI.
        */
-      varst = safe_asprintf (g, "%s/vars.fd.%d", g->tmpdir,
++g->unique);
+      varst = guestfs_int_make_temp_path (g, "vars", "fd");
+      if (!varst)
+        return -1;
       guestfs_int_cmd_add_arg (copycmd, "cp");
       guestfs_int_cmd_add_arg (copycmd, varsfile);
       guestfs_int_cmd_add_arg (copycmd, varst);
diff --git a/lib/command.c b/lib/command.c
index bc469de59..bfec76f19 100644
--- a/lib/command.c
+++ b/lib/command.c
@@ -812,11 +812,9 @@ guestfs_int_cmd_pipe_run (struct command *cmd, const char
*mode)
    * we write them into a temporary file and provide a separate
    * function for the caller to read the error messages.
    */
-  if (guestfs_int_lazy_make_tmpdir (cmd->g) == -1)
+  cmd->error_file = guestfs_int_make_temp_path (cmd->g,
"cmderr", "txt");
+  if (!cmd->error_file)
     goto error;
-
-  cmd->error_file -    safe_asprintf (cmd->g, "%s/cmderr.%d",
cmd->g->tmpdir, ++cmd->g->unique);
   errfd = open (cmd->error_file,
                 O_WRONLY|O_CREAT|O_NOCTTY|O_TRUNC|O_CLOEXEC, 0600);
   if (errfd == -1) {
diff --git a/lib/drives.c b/lib/drives.c
index 117c8bf85..82ef30093 100644
--- a/lib/drives.c
+++ b/lib/drives.c
@@ -987,9 +987,9 @@ guestfs_impl_add_drive_scratch (guestfs_h *g, int64_t size,
    * because everything in g->tmpdir is 'rm -rf'd when the handle is
    * closed.
    */
-  if (guestfs_int_lazy_make_tmpdir (g) == -1)
+  filename = guestfs_int_make_temp_path (g, "scratch",
"img");
+  if (!filename)
     return -1;
-  filename = safe_asprintf (g, "%s/scratch.%d", g->tmpdir,
++g->unique);
 
   /* Create a raw format temporary disk. */
   if (guestfs_disk_create (g, filename, "raw", size, -1) == -1)
diff --git a/lib/inspect-icon.c b/lib/inspect-icon.c
index 3e44b23eb..533e9fb4b 100644
--- a/lib/inspect-icon.c
+++ b/lib/inspect-icon.c
@@ -390,7 +390,9 @@ icon_cirros (guestfs_h *g, size_t *size_r)
     return NOT_FOUND;
 
   /* Use pbmtext to render it. */
-  pngfile = safe_asprintf (g, "%s/cirros.png", g->tmpdir);
+  pngfile = guestfs_int_make_temp_path (g, "cirros",
"png");
+  if (!pngfile)
+    return NOT_FOUND;
 
   guestfs_int_cmd_add_string_unquoted (cmd, PBMTEXT " < ");
   guestfs_int_cmd_add_string_quoted   (cmd, local);
@@ -474,7 +476,9 @@ icon_windows_xp (guestfs_h *g, const char *systemroot,
size_t *size_r)
   if (filename_downloaded == NULL)
     return NOT_FOUND;
 
-  pngfile = safe_asprintf (g, "%s/windows-xp-icon.png",
g->tmpdir);
+  pngfile = guestfs_int_make_temp_path (g, "windows-xp-icon",
"png");
+  if (!pngfile)
+    return NOT_FOUND;
 
   guestfs_int_cmd_add_string_unquoted (cmd, WRESTOOL " -x --type=2
--name=143 ");
   guestfs_int_cmd_add_string_quoted   (cmd, filename_downloaded);
@@ -542,7 +546,9 @@ icon_windows_7 (guestfs_h *g, const char *systemroot, size_t
*size_r)
   if (filename_downloaded == NULL)
     return NOT_FOUND;
 
-  pngfile = safe_asprintf (g, "%s/windows-7-icon.png", g->tmpdir);
+  pngfile = guestfs_int_make_temp_path (g, "windows-7-icon",
"png");
+  if (!pngfile)
+    return NOT_FOUND;
 
   guestfs_int_cmd_add_string_unquoted (cmd,
                                        WRESTOOL " -x --type=2 --name=6801
");
diff --git a/lib/launch-direct.c b/lib/launch-direct.c
index 00cb25077..87ac121c7 100644
--- a/lib/launch-direct.c
+++ b/lib/launch-direct.c
@@ -86,11 +86,10 @@ create_cow_overlay_direct (guestfs_h *g, void *datav, struct
drive *drv)
   if (!backing_drive)
     return NULL;
 
-  if (guestfs_int_lazy_make_tmpdir (g) == -1)
+  overlay = guestfs_int_make_temp_path (g, "overlay",
"qcow2");
+  if (!overlay)
     return NULL;
 
-  overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir,
++g->unique);
-
   optargs.bitmask = GUESTFS_DISK_CREATE_BACKINGFILE_BITMASK;
   optargs.backingfile = backing_drive;
   if (drv->src.format) {
diff --git a/lib/launch-libvirt.c b/lib/launch-libvirt.c
index 98c947288..e5121799e 100644
--- a/lib/launch-libvirt.c
+++ b/lib/launch-libvirt.c
@@ -224,11 +224,10 @@ make_qcow2_overlay (guestfs_h *g, const char
*backing_drive,
   char *overlay;
   struct guestfs_disk_create_argv optargs;
 
-  if (guestfs_int_lazy_make_tmpdir (g) == -1)
+  overlay = guestfs_int_make_temp_path (g, "overlay",
"qcow2");
+  if (!overlay)
     return NULL;
 
-  overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir,
++g->unique);
-
   optargs.bitmask = GUESTFS_DISK_CREATE_BACKINGFILE_BITMASK;
   optargs.backingfile = backing_drive;
   if (format) {
diff --git a/lib/launch-uml.c b/lib/launch-uml.c
index 7c2b276d8..db68fac7b 100644
--- a/lib/launch-uml.c
+++ b/lib/launch-uml.c
@@ -54,11 +54,10 @@ make_cow_overlay (guestfs_h *g, const char *original)
   char *overlay;
   int r;
 
-  if (guestfs_int_lazy_make_tmpdir (g) == -1)
+  overlay = guestfs_int_make_temp_path (g, "overlay",
"qcow2");
+  if (!overlay)
     return NULL;
 
-  overlay = safe_asprintf (g, "%s/overlay%d", g->tmpdir,
g->unique++);
-
   guestfs_int_cmd_add_arg (cmd, "uml_mkcow");
   guestfs_int_cmd_add_arg (cmd, overlay);
   guestfs_int_cmd_add_arg (cmd, original);
-- 
2.13.2

On Wednesday, 20 September 2017 18:57:11 CEST Richard W.M. Jones
wrote:
> v1 -> v2:
> 
>  - Remove the unnecessary calls to guestfs_int_lazy_make_tmpdir
>    in the final patch.
The series LGTM.

Thanks,
-- 
Pino Toscano