threads / discuss / 22817

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

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

## tl;dr

2 messages between Feb 25, 2010 and Feb 25, 2010.

replies: 1people: 2as markdown or json

Filippo Negroni· Feb 25, 2010, 09:01 UTC · lore
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.
>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
Matthieu Moy· Feb 25, 2010, 09:31 UTC · re: Filippo Negroni · lore

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

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/

← back to recent threads