Libguestfs - Aug 2009 - [PATCH libguestfs] generator.ml: constify do_mkdtemp

Not useful from a testing standpoint (since this presumes
a metric ton of other changes), but mostly a heads-up where I'm going.

>From 62cbebffe9859046019b500e4bcdb0e44e575755 Mon Sep 17 00:00:00 2001
From: Jim Meyering <meyering at redhat.com>
Date: Tue, 11 Aug 2009 17:17:08 +0200
Subject: [PATCH libguestfs] generator.ml: constify do_mkdtemp

* daemon/dir.c (do_mkdtemp): Rewrite for a "const" parameter.
* src/generator.ml (mkdtemp): Declare parameter to be of type Pathname.
---
 daemon/dir.c     |   27 ++++++++-------------------
 src/generator.ml |    8 +++-----
 2 files changed, 11 insertions(+), 24 deletions(-)

diff --git a/daemon/dir.c b/daemon/dir.c
index ad1c7c9..21ae07e 100644
--- a/daemon/dir.c
+++ b/daemon/dir.c
@@ -185,31 +185,20 @@ do_is_dir (const char *path)
 }

 char *
-do_mkdtemp (char *template)
+do_mkdtemp (const char *template)
 {
-  char *r;
-
-  NEED_ROOT (return NULL);
-  ABS_PATH (template, return NULL);
+  char *r = strdup (template);
+  if (r == NULL) {
+    reply_with_perror ("strdup");
+    return NULL;
+  }

   CHROOT_IN;
-  r = mkdtemp (template);
+  r = mkdtemp (r);
   CHROOT_OUT;

-  if (r == NULL) {
+  if (r == NULL)
     reply_with_perror ("mkdtemp: %s", template);
-    return NULL;
-  }

-  /* The caller will free template AND try to free the return value,
-   * so we must make a copy here.
-   */
-  if (r == template) {
-    r = strdup (template);
-    if (r == NULL) {
-      reply_with_perror ("strdup");
-      return NULL;
-    }
-  }
   return r;
 }
diff --git a/src/generator.ml b/src/generator.ml
index 8dc9261..3c7512f 100755
--- a/src/generator.ml
+++ b/src/generator.ml
@@ -2594,8 +2594,7 @@ containing C<dir>.
 It is an interface to the L<scrub(1)> program.  See that
 manual page for more details.");

-(* FIXME: make this a WritableString? *)
-  ("mkdtemp", (RString "dir", [String
"template"]), 117, [],
+  ("mkdtemp", (RString "dir", [Pathname
"template"]), 117, [],
    [InitBasicFS, Always, TestRun (
       [["mkdir"; "/tmp"];
        ["mkdtemp"; "/tmp/tmpXXXXXX"]])],

On Tue, Aug 11, 2009 at 05:20:37PM +0200, Jim Meyering
wrote:
> Not useful from a testing standpoint (since this presumes
> a metric ton of other changes), but mostly a heads-up where I'm going.
> +  char *r = strdup (template);
> +  if (r == NULL) {
> +    reply_with_perror ("strdup");
> +    return NULL;
> +  }
> 
>    CHROOT_IN;
> -  r = mkdtemp (template);
> +  r = mkdtemp (r);
>    CHROOT_OUT;
> 
> -  if (r == NULL) {
> +  if (r == NULL)
>      reply_with_perror ("mkdtemp: %s", template);
> -    return NULL;
> -  }
> 
> -  /* The caller will free template AND try to free the return value,
> -   * so we must make a copy here.
> -   */
> -  if (r == template) {
> -    r = strdup (template);
> -    if (r == NULL) {
> -      reply_with_perror ("strdup");
> -      return NULL;
> -    }
> -  }
>    return r;
I think 'r' gets leaked along the error path (when mkdtemp returns
NULL).

Rich.

-- 
Richard Jones, Emerging Technologies, Red Hat  http://et.redhat.com/~rjones
virt-df lists disk usage of guests without needing to install any
software inside the virtual machine.  Supports Linux and Windows.
http://et.redhat.com/~rjones/virt-df/