threads / patch / 16414

patchsha1_file: avoid bogus "file exists" error message

Subject: [PATCH] sha1_file: avoid bogus "file exists" error message

## tl;dr

4 messages between Nov 20, 2008 and Nov 28, 2008. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Joey Hess· Nov 20, 2008, 18:56 UTC · lore

This avoids the following misleading error message:

error: unable to create temporary sha1 filename ./objects/15: File exists

mkstemp can fail for many reasons, one of which, ENOENT, can occur if the directory for the temp file doesn't exist. create_tmpfile tried to handle this case by always trying to mkdir the directory, even if it already existed. This caused errno to be clobbered, so one cannot tell why mkstemp really failed, and it truncated the buffer to just the directory name, resulting in the strange error message shown above.

Note that in both occasions that I've seen this failure, it has not been due to a missing directory, or bad permissions, but some other, unknown mkstemp failure mode that did not occur when I ran git again. This code could perhaps be made more robust by retrying mkstemp, in case it was a transient failure.

Signed-off-by: Joey Hess <joey@kitenet.net>
---
 sha1_file.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

Show changes to sha1_file.c +1 −1

diff --git a/sha1_file.c b/sha1_file.c
index ab2b520..927fb64 100644
--- a/sha1_file.c
+++ b/sha1_file.c
@@ -2231,7 +2231,7 @@ static int create_tmpfile(char *buffer, size_t bufsiz, const char *filename)
 	memcpy(buffer, filename, dirlen);
 	strcpy(buffer + dirlen, "tmp_obj_XXXXXX");
 	fd = mkstemp(buffer);
-	if (fd < 0 && dirlen) {
+	if (fd < 0 && dirlen && errno == ENOENT) {
 		/* Make sure the directory exists */
 		memcpy(buffer, filename, dirlen);
 		buffer[dirlen-1] = 0;

-- 
1.5.6.5

Joey Hess· Nov 26, 2008, 18:19 UTC · re: Joey Hess · lore

Re: [PATCH] sha1_file: avoid bogus "file exists" error message

Joey Hess wrote:

> Note that in both occasions that I've seen this failure, it has not been
> due to a missing directory, or bad permissions

Actually, it was due to bad permissions. :-) Once git was fixed to actually say that, I figured out where to look to fix them.

-- 
see shy jo

Ian Hilt· Nov 27, 2008, 17:41 UTC · re: Joey Hess · lore

Re: [PATCH] sha1_file: avoid bogus "file exists" error message

On Wed, 26 Nov 2008, Joey Hess wrote:

Show 6 quoted lines

> Joey Hess wrote:
> > Note that in both occasions that I've seen this failure, it has not been
> > due to a missing directory, or bad permissions
> 
> Actually, it was due to bad permissions. :-) Once git was fixed to
> actually say that, I figured out where to look to fix them.

This is strange since write_loose_object() which calls create_tmpfile() checks for EPERM. Perhaps this should be done in create_tmpfile()?

Joey Hess· Nov 28, 2008, 17:00 UTC · re: Ian Hilt · lore

Re: [PATCH] sha1_file: avoid bogus "file exists" error message

Ian Hilt wrote:

Show 10 quoted lines

> On Wed, 26 Nov 2008, Joey Hess wrote:
> > Joey Hess wrote:
> > > Note that in both occasions that I've seen this failure, it has not been
> > > due to a missing directory, or bad permissions
> > 
> > Actually, it was due to bad permissions. :-) Once git was fixed to
> > actually say that, I figured out where to look to fix them.
> 
> This is strange since write_loose_object() which calls create_tmpfile()
> checks for EPERM.  Perhaps this should be done in create_tmpfile()?

errno is clobbered by the mkdir in create_tmpfile(), that's what my patch corrects.

I suspect that in my case, mkstemp failed with EACCES, not EPERM. git was running as a group that did not have write access to (some) object directories.

-- 
see shy jo