Libguestfs - Jul 2014 - [PATCH] make-fs: do not leak the fd from mkstemp

Do not hardcode /tmp.
---
 make-fs/make-fs.c | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/make-fs/make-fs.c b/make-fs/make-fs.c
index 1bec3e2..9c11fef 100644
--- a/make-fs/make-fs.c
+++ b/make-fs/make-fs.c
@@ -381,8 +381,9 @@ estimate_input (const char *input, uint64_t *estimate_rtn,
char **ifmt_rtn)
   CLEANUP_FCLOSE FILE *fp = NULL;
   char line[256];
   size_t len;
+  CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
 
-  if (asprintf (&tmpfile, "/tmp/makefsXXXXXX") == -1) {
+  if (asprintf (&tmpfile, "%s/makefsXXXXXX", tmpdir) == -1) {
     perror ("asprintf");
     return -1;
   }
-- 
1.9.3

---
 make-fs/make-fs.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/make-fs/make-fs.c b/make-fs/make-fs.c
index 9c11fef..406aa1d 100644
--- a/make-fs/make-fs.c
+++ b/make-fs/make-fs.c
@@ -382,15 +382,18 @@ estimate_input (const char *input, uint64_t *estimate_rtn,
char **ifmt_rtn)
   char line[256];
   size_t len;
   CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
+  int fd;
 
   if (asprintf (&tmpfile, "%s/makefsXXXXXX", tmpdir) == -1) {
     perror ("asprintf");
     return -1;
   }
-  if (mkstemp (tmpfile) == -1) {
+  fd = mkstemp (tmpfile);
+  if (fd == -1) {
     perror (tmpfile);
     return -1;
   }
+  close (fd);
 
   if (stat (input, &statbuf) == -1) {
     perror (input);
-- 
1.9.3

Some protocols may pass a null path to make_uri, so make sure to handle
that gracefully.
---
 src/launch-direct.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/src/launch-direct.c b/src/launch-direct.c
index 104809d..9654e04 100644
--- a/src/launch-direct.c
+++ b/src/launch-direct.c
@@ -1218,7 +1218,7 @@ make_uri (guestfs_h *g, const char *scheme, const char
*user,
   CLEANUP_FREE char *userauth = NULL;
 
   /* Need to add a leading '/' to URI paths since xmlSaveUri
doesn't. */
-  if (path[0] != '/') {
+  if (path != NULL && path[0] != '/') {
     pathslash = safe_asprintf (g, "/%s", path);
     uri.path = pathslash;
   }
-- 
1.9.3

On Mon, Jul 28, 2014 at 07:53:19PM +0200, Pino Toscano
wrote:
> Do not hardcode /tmp.
> ---
>  make-fs/make-fs.c | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/make-fs/make-fs.c b/make-fs/make-fs.c
> index 1bec3e2..9c11fef 100644
> --- a/make-fs/make-fs.c
> +++ b/make-fs/make-fs.c
> @@ -381,8 +381,9 @@ estimate_input (const char *input, uint64_t
*estimate_rtn, char **ifmt_rtn)
>    CLEANUP_FCLOSE FILE *fp = NULL;
>    char line[256];
>    size_t len;
> +  CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
>  
> -  if (asprintf (&tmpfile, "/tmp/makefsXXXXXX") == -1) {
> +  if (asprintf (&tmpfile, "%s/makefsXXXXXX", tmpdir) == -1)
{
>      perror ("asprintf");
>      return -1;
>    }
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://people.redhat.com/~rjones/virt-df/

On Mon, Jul 28, 2014 at 07:53:20PM +0200, Pino Toscano
wrote:
> ---
>  make-fs/make-fs.c | 5 ++++-
>  1 file changed, 4 insertions(+), 1 deletion(-)
> 
> diff --git a/make-fs/make-fs.c b/make-fs/make-fs.c
> index 9c11fef..406aa1d 100644
> --- a/make-fs/make-fs.c
> +++ b/make-fs/make-fs.c
> @@ -382,15 +382,18 @@ estimate_input (const char *input, uint64_t
*estimate_rtn, char **ifmt_rtn)
>    char line[256];
>    size_t len;
>    CLEANUP_FREE char *tmpdir = guestfs_get_tmpdir (g);
> +  int fd;
>  
>    if (asprintf (&tmpfile, "%s/makefsXXXXXX", tmpdir) == -1)
{
>      perror ("asprintf");
>      return -1;
>    }
> -  if (mkstemp (tmpfile) == -1) {
> +  fd = mkstemp (tmpfile);
> +  if (fd == -1) {
>      perror (tmpfile);
>      return -1;
>    }
> +  close (fd);
>  
>    if (stat (input, &statbuf) == -1) {
>      perror (input);
ACK.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
Read my programming and virtualization blog: http://rwmj.wordpress.com
virt-p2v converts physical machines to virtual machines.  Boot with a
live CD or over the network (PXE) and turn machines into KVM guests.
http://libguestfs.org/virt-v2v

On Mon, Jul 28, 2014 at 07:53:21PM +0200, Pino Toscano
wrote:
> Some protocols may pass a null path to make_uri, so make sure to handle
> that gracefully.
> ---
>  src/launch-direct.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/src/launch-direct.c b/src/launch-direct.c
> index 104809d..9654e04 100644
> --- a/src/launch-direct.c
> +++ b/src/launch-direct.c
> @@ -1218,7 +1218,7 @@ make_uri (guestfs_h *g, const char *scheme, const
char *user,
>    CLEANUP_FREE char *userauth = NULL;
>  
>    /* Need to add a leading '/' to URI paths since xmlSaveUri
doesn't. */
> -  if (path[0] != '/') {
> +  if (path != NULL && path[0] != '/') {
>      pathslash = safe_asprintf (g, "/%s", path);
>      uri.path = pathslash;
>    }
Slightly hard to follow the original code, but I think this is correct
so ACK, especially if it passes the test (assuming we have a test
which covers this obscure gluster+unix case ...).

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