[PATCH/RFC] Bugfix for AIX tmpfile creation

[PATCH/RFC] Bugfix for AIX tmpfile creation

[Patrick Higgins]

The AIX mkstemp will modify it's template parameter to an empty string if
the call fails. This caused a subsequent mkdir to fail.

Signed-off-by: Patrick Higgins <patrick.higgins@cexp.com>
---
 sha1_file.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/sha1_file.c b/sha1_file.c
index 191f814..92299ed 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2118,6 +2118,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
 	fd = mkstemp(buffer);
 	if (fd < 0 && dirlen) {
 		/* Make sure the directory exists */
+		memcpy(buffer, filename, dirlen);
 		buffer[dirlen-1] = 0;
 		if (mkdir(buffer, 0777) || adjust_shared_perm(buffer))
 			return -1;
-- 
1.5.6.dirty

Re: [PATCH/RFC] Bugfix for AIX tmpfile creation

[Junio C Hamano]

Hmm, we have quite a few calls to mkstemp(), so adding the workaround to
this specific call site does not scale well as a fix, I am afraid.

Some callers do not want to use the xmkstemp() (because the function
always dies upon failure), so working around this bug in xmkstemp() won't
solve it either, unfortunately.  We would probably need compat/mkstemp.c
that wraps problematic mkstemp() on platforms with broken mkstemp()
implementation.

Re: [PATCH/RFC] Bugfix for AIX tmpfile creation

[Linus Torvalds]

On Mon, 23 Jun 2008, Junio C Hamano wrote:
>
> Hmm, we have quite a few calls to mkstemp(), so adding the workaround to
> this specific call site does not scale well as a fix, I am afraid.

All other callers of mkstemp() always rewrite the _entire_ path and don't 
care about the result after failure (since it failed)

The new 'create_tmpfile()' usage was/is special in that it re-uses the 
path without recreating it all, just the final part.

So I think Patrick's patch is fine and sufficient. Maybe I missed some in 
my quick grep, but it does look ok, and the create_tmpfile() usage really 
was pretty special.

		Linus

RE: [PATCH/RFC] Bugfix for AIX tmpfile creation

[Patrick.Higgins]

On Behalf Of Linus Torvalds
> To: Junio C Hamano
> 
> On Mon, 23 Jun 2008, Junio C Hamano wrote:
> >
> > Hmm, we have quite a few calls to mkstemp(), so adding the 
> workaround to
> > this specific call site does not scale well as a fix, I am afraid.
> 
> All other callers of mkstemp() always rewrite the _entire_ 
> path and don't 
> care about the result after failure (since it failed)
> 
> The new 'create_tmpfile()' usage was/is special in that it 
> re-uses the 
> path without recreating it all, just the final part.
> 
> So I think Patrick's patch is fine and sufficient. Maybe I 
> missed some in 
> my quick grep, but it does look ok, and the create_tmpfile() 
> usage really 
> was pretty special.

This is the only problem I've seen with it so far on AIX. I was really surprised because I hadn't seen any problems with any of the release candidates and had been using them all as they came out so I got lazy and didn't test the final release, but one of my co-workers let me know that it is broken this morning. Even the most basic things like git-add stopped working, and applying this small patch fixed all test cases. The irony is of course that I wanted 1.5.6 for the improved AIX support and the final release is completely broken on AIX!

Re: [PATCH/RFC] Bugfix for AIX tmpfile creation

[Junio C Hamano]

<Patrick.Higgins@cexp.com> writes:

> The irony is of course that I wanted 1.5.6 for the improved AIX support
> and the final release is completely broken on AIX!

Yeah, queued to 'maint' for 1.5.6.1.  Thanks

Re: [PATCH/RFC] Bugfix for AIX tmpfile creation

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> On Mon, 23 Jun 2008, Junio C Hamano wrote:
>>
>> Hmm, we have quite a few calls to mkstemp(), so adding the workaround to
>> this specific call site does not scale well as a fix, I am afraid.
>
> All other callers of mkstemp() always rewrite the _entire_ path and don't 
> care about the result after failure (since it failed)

Yeah, I realize I grepped wrong X-<.