Re: [PATCH v2 1/2] compat: add a mkstemps() compatibility function

From: Johannes Sixt <j.sixt@viscovery.net>
Date: May 28, 2009, 07:07 UTC
Message-ID: <4A1E3834.3020502@viscovery.net>
In-Reply-To: <1243491077-27738-1-git-send-email-davvid@gmail.com>

David Aguilar schrieb:

Show 10 quoted lines

> +++ b/compat/mkstemps.c
> @@ -0,0 +1,78 @@
> +#include <string.h>
> +#include <errno.h>
> +#include <stdio.h>
> +#include <fcntl.h>
> +#include <inttypes.h>
> +#include <unistd.h>
> +#include <sys/time.h>
> +#include <sys/types.h>

...

> +#ifndef O_BINARY
> +#define O_BINARY 0
> +#endif

...

> +		fd = open(pattern, O_BINARY|O_CREAT|O_EXCL|O_RDWR, 0600);

You should not include "random" system headers, nor has mkstemps any business deciding whether files are opened in binary mode. We are not using O_BINARY anywhere else (except in compat/mingw.c). With the patch below squashed in (I hope it won't be wrapped) you can add:

Tested-by: Johannes Sixt <j6t@kdbg.org> (Windows)

to both your patches. And, yes, I like them :-)

-- Hannes

diff --git a/compat/mkstemps.c b/compat/mkstemps.c
index 10f9ed6..1cf7f3d 100644
--- a/compat/mkstemps.c
+++ b/compat/mkstemps.c
@@ -1,20 +1,9 @@
-#include <string.h>
-#include <errno.h>
-#include <stdio.h>
-#include <fcntl.h>
-#include <inttypes.h>
-#include <unistd.h>
-#include <sys/time.h>
-#include <sys/types.h>
+#include "../git-compat-util.h"

 #ifndef TMP_MAX
 #define TMP_MAX 16384
 #endif

-#ifndef O_BINARY
-#define O_BINARY 0
-#endif
-
 /* Adapted from libiberty's mkstemp.c. */
 int gitmkstemps(char *pattern, int suffix_len)
 {
@@ -57,7 +46,7 @@ int gitmkstemps(char *pattern, int suffix_len)
 		template[4] = letters[v % num_letters]; v/= num_letters;
 		template[5] = letters[v % num_letters]; v/= num_letters;

-		fd = open(pattern, O_BINARY|O_CREAT|O_EXCL|O_RDWR, 0600);
+		fd = open(pattern, O_CREAT|O_EXCL|O_RDWR, 0600);
 		if (fd > 0)
 			return fd;
 		/* Fatal error (EPERM, ENOSPC etc).