compat/mkdtemp.c: patch for gitmkdtemp() compatibility implementation

compat/mkdtemp.c: patch for gitmkdtemp() compatibility implementation

[Filippo Negroni]

[-- Attachment #1: Type: text/plain, Size: 1234 bytes --]

Dear all,

I noticed that the current implementation of mkdtemp() in the compat/ directory does not seem to be correct.

gitmkdtemp() is needed for platforms such as Solaris 9 and 10 which do not implement mkdtemp().

The current implementation (compat/mkdtemp.c):

----
char *gitmkdtemp(char *template)
{
        if (!mktemp(template) || mkdir(template, 0700))
                return NULL;
        return template;
}
----

seems to be testing the return value of mktemp incorrectly.

mktemp() always returns the argument 'template', even on failure.

This is an extract from Solaris's mktemp(2) manual page:

RETURN VALUES
     The mktemp() function will  assign  to  template  the  empty
     string if it  cannot create a unique name.

Upon failure, 'template' is set to an empty string, i.e., it's first character is '\0'.

The fixed implementation should read:

----
char *gitmkdtemp(char *template)
{
        if (!*mktemp(template) || mkdir(template, 0700))
                return NULL;
        return template;
}
----

I.e., simply add the pointer dereference operator when calling mktemp().

I have attached a patch against the most current git repo, branch master.

Please let me know if you need the patch in any other way.

[-- Attachment #2: 0001-Fixed-return-value-check-for-mkdtemp-implementation.patch --]
[-- Type: text/plain, Size: 703 bytes --]

>From 00b2a96d93d801d06eb710bc03b3e4804d3583ec Mon Sep 17 00:00:00 2001
From: Filippo Negroni <fnegroni@flexerasoftware.com>
Date: Thu, 25 Feb 2010 08:39:35 +0000
Subject: [PATCH] Fixed return value check for mkdtemp implementation.

mktemp() returns an 'empty' string, not a NULL pointer.
---
 compat/mkdtemp.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/compat/mkdtemp.c b/compat/mkdtemp.c
index 34d4b49..1136119 100644
--- a/compat/mkdtemp.c
+++ b/compat/mkdtemp.c
@@ -2,7 +2,7 @@
 
 char *gitmkdtemp(char *template)
 {
-	if (!mktemp(template) || mkdir(template, 0700))
+	if (!*mktemp(template) || mkdir(template, 0700))
 		return NULL;
 	return template;
 }
-- 
1.6.4

Re: compat/mkdtemp.c: patch for gitmkdtemp() compatibility implementation

[Matthieu Moy]

Thanks for your contribution to git. However ...

Filippo Negroni <fnegroni@flexerasoftware.com> writes:

> Please let me know if you need the patch in any other way.

Please, read

http://repo.or.cz/w/git.git?a=blob;f=Documentation/SubmittingPatches;hb=HEAD

especially about inlining the patch and Signed-off-by, to make it easy
and legally possible for our maintainer to apply your patch.

Thanks,

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/