threads / rfc / 14107

RFC patchBugfix for AIX tmpfile creation

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

## tl;dr

6 messages between Jun 23, 2008 and Jun 23, 2008. Diffs are folded; open one to read it.

replies: 5people: 3as markdown or json

Patrick Higgins· Jun 23, 2008, 21:33 UTC · lore

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(-)

Show changes to sha1_file.c +1 −0

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

Junio C Hamano· Jun 23, 2008, 22:29 UTC · re: Patrick Higgins · lore

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

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.

Linus Torvalds· Jun 23, 2008, 22:55 UTC · re: Junio C Hamano · lore

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

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

patrick.higgins@cexp.com· Jun 23, 2008, 23:07 UTC · re: Linus Torvalds · lore

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

On Behalf Of Linus Torvalds

Show 21 quoted lines

> 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!

Junio C Hamano· Jun 23, 2008, 23:14 UTC · re: patrick.higgins@cexp.com · lore

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

<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

Junio C Hamano· Jun 23, 2008, 23:10 UTC · re: Linus Torvalds · lore

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

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

Show 7 quoted lines

> 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-<.