* [PATCH v2 1/2] compat: add a mkstemps() compatibility function @ 2009-05-28 6:11 David Aguilar 2009-05-28 6:11 ` [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF David Aguilar 2009-05-28 7:07 ` [PATCH v2 1/2] compat: add a mkstemps() compatibility function Johannes Sixt 0 siblings, 2 replies; 7+ messages in thread From: David Aguilar @ 2009-05-28 6:11 UTC (permalink / raw) To: git; +Cc: gitster, markus.heidelberg, jnareb, j.sixt, David Aguilar mkstemps() is a BSD extension so provide an implementation for cross-platform use. Signed-off-by: David Aguilar <davvid@gmail.com> --- This includes Jakub's autoconf fixups. Makefile | 19 +++++++++++++ compat/mkstemps.c | 78 +++++++++++++++++++++++++++++++++++++++++++++++++++++ config.mak.in | 1 + configure.ac | 6 ++++ git-compat-util.h | 5 +++ 5 files changed, 109 insertions(+), 0 deletions(-) create mode 100644 compat/mkstemps.c diff --git a/Makefile b/Makefile index eaae45d..a70b5f0 100644 --- a/Makefile +++ b/Makefile @@ -52,6 +52,8 @@ all:: # # Define NO_MKDTEMP if you don't have mkdtemp in the C library. # +# Define NO_MKSTEMPS if you don't have mkstemps in the C library. +# # Define NO_SYS_SELECT_H if you don't have sys/select.h. # # Define NO_SYMLINK_HEAD if you never want .git/HEAD to be a symbolic link. @@ -636,10 +638,12 @@ EXTLIBS = ifeq ($(uname_S),Linux) NO_STRLCPY = YesPlease + NO_MKSTEMPS = YesPlease THREADED_DELTA_SEARCH = YesPlease endif ifeq ($(uname_S),GNU/kFreeBSD) NO_STRLCPY = YesPlease + NO_MKSTEMPS = YesPlease THREADED_DELTA_SEARCH = YesPlease endif ifeq ($(uname_S),UnixWare) @@ -651,6 +655,7 @@ ifeq ($(uname_S),UnixWare) SHELL_PATH = /usr/local/bin/bash NO_IPV6 = YesPlease NO_HSTRERROR = YesPlease + NO_MKSTEMPS = YesPlease BASIC_CFLAGS += -Kthread BASIC_CFLAGS += -I/usr/local/include BASIC_LDFLAGS += -L/usr/local/lib @@ -674,6 +679,7 @@ ifeq ($(uname_S),SCO_SV) SHELL_PATH = /usr/bin/bash NO_IPV6 = YesPlease NO_HSTRERROR = YesPlease + NO_MKSTEMPS = YesPlease BASIC_CFLAGS += -I/usr/local/include BASIC_LDFLAGS += -L/usr/local/lib NO_STRCASESTR = YesPlease @@ -702,6 +708,7 @@ ifeq ($(uname_S),SunOS) NO_MEMMEM = YesPlease NO_HSTRERROR = YesPlease NO_MKDTEMP = YesPlease + NO_MKSTEMPS = YesPlease OLD_ICONV = UnfortunatelyYes ifeq ($(uname_R),5.8) NO_UNSETENV = YesPlease @@ -724,6 +731,7 @@ ifeq ($(uname_O),Cygwin) NO_D_INO_IN_DIRENT = YesPlease NO_STRCASESTR = YesPlease NO_MEMMEM = YesPlease + NO_MKSTEMPS = YesPlease NO_SYMLINK_HEAD = YesPlease NEEDS_LIBICONV = YesPlease NO_FAST_WORKING_DIRECTORY = UnfortunatelyYes @@ -767,11 +775,13 @@ ifeq ($(uname_S),NetBSD) BASIC_LDFLAGS += -L/usr/pkg/lib $(CC_LD_DYNPATH)/usr/pkg/lib THREADED_DELTA_SEARCH = YesPlease USE_ST_TIMESPEC = YesPlease + NO_MKSTEMPS = YesPlease endif ifeq ($(uname_S),AIX) NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease NO_MKDTEMP = YesPlease + NO_MKSTEMPS = YesPlease NO_STRLCPY = YesPlease NO_NSEC = YesPlease FREAD_READS_DIRECTORIES = UnfortunatelyYes @@ -787,12 +797,14 @@ endif ifeq ($(uname_S),GNU) # GNU/Hurd NO_STRLCPY=YesPlease + NO_MKSTEMPS = YesPlease endif ifeq ($(uname_S),IRIX64) NO_IPV6=YesPlease NO_SETENV=YesPlease NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease + NO_MKSTEMPS = YesPlease NO_STRLCPY = YesPlease NO_SOCKADDR_STORAGE=YesPlease SHELL_PATH=/usr/gnu/bin/bash @@ -805,6 +817,7 @@ ifeq ($(uname_S),HP-UX) NO_SETENV=YesPlease NO_STRCASESTR=YesPlease NO_MEMMEM = YesPlease + NO_MKSTEMPS = YesPlease NO_STRLCPY = YesPlease NO_MKDTEMP = YesPlease NO_UNSETENV = YesPlease @@ -834,6 +847,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) NO_C99_FORMAT = YesPlease NO_STRTOUMAX = YesPlease NO_MKDTEMP = YesPlease + NO_MKSTEMPS = YesPlease SNPRINTF_RETURNS_BOGUS = YesPlease NO_SVN_TESTS = YesPlease NO_PERL_MAKEMAKER = YesPlease @@ -853,6 +867,7 @@ ifneq (,$(findstring MINGW,$(uname_S))) endif ifneq (,$(findstring arm,$(uname_M))) ARM_SHA1 = YesPlease + NO_MKSTEMPS = YesPlease endif -include config.mak.autogen @@ -1011,6 +1026,10 @@ ifdef NO_MKDTEMP COMPAT_CFLAGS += -DNO_MKDTEMP COMPAT_OBJS += compat/mkdtemp.o endif +ifdef NO_MKSTEMPS + COMPAT_CFLAGS += -DNO_MKSTEMPS + COMPAT_OBJS += compat/mkstemps.o +endif ifdef NO_UNSETENV COMPAT_CFLAGS += -DNO_UNSETENV COMPAT_OBJS += compat/unsetenv.o diff --git a/compat/mkstemps.c b/compat/mkstemps.c new file mode 100644 index 0000000..10f9ed6 --- /dev/null +++ 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 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) +{ + static const char letters[] = + "abcdefghijklmnopqrstuvwxyz" + "ABCDEFGHIJKLMNOPQRSTUVWXYZ" + "0123456789"; + static const int num_letters = 62; + uint64_t value; + struct timeval tv; + char *template; + size_t len; + int fd, count; + + len = strlen(pattern); + + if (len < 6 + suffix_len) { + errno = EINVAL; + return -1; + } + + if (strncmp(&pattern[len - 6 - suffix_len], "XXXXXX", 6)) { + errno = EINVAL; + return -1; + } + + /* Replace pattern's XXXXXX characters with randomness. + * Try TMP_MAX different filenames. + */ + gettimeofday(&tv, NULL); + value = ((size_t)(tv.tv_usec << 16)) ^ tv.tv_sec ^ getpid(); + template = &pattern[len - 6 - suffix_len]; + for (count = 0; count < TMP_MAX; ++count) { + uint64_t v = value; + /* Fill in the random bits. */ + template[0] = letters[v % num_letters]; v/= num_letters; + template[1] = letters[v % num_letters]; v/= num_letters; + template[2] = letters[v % num_letters]; v/= num_letters; + template[3] = letters[v % num_letters]; v/= num_letters; + 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); + if (fd > 0) + return fd; + /* Fatal error (EPERM, ENOSPC etc). + * It doesn't make sense to loop. + */ + if (errno != EEXIST) + break; + /* This is a random value. It is only necessary that + * the next TMP_MAX values generated by adding 7777 to + * VALUE are different with (module 2^32). + */ + value += 7777; + } + /* We return the null string if we can't find a unique file name. */ + pattern[0] = '\0'; + errno = EINVAL; + return -1; +} diff --git a/config.mak.in b/config.mak.in index 7cce0c1..b6619af 100644 --- a/config.mak.in +++ b/config.mak.in @@ -46,6 +46,7 @@ NO_STRTOUMAX=@NO_STRTOUMAX@ NO_SETENV=@NO_SETENV@ NO_UNSETENV=@NO_UNSETENV@ NO_MKDTEMP=@NO_MKDTEMP@ +NO_MKSTEMPS=@NO_MKSTEMPS@ NO_ICONV=@NO_ICONV@ OLD_ICONV=@OLD_ICONV@ NO_DEFLATE_BOUND=@NO_DEFLATE_BOUND@ diff --git a/configure.ac b/configure.ac index 4e728bc..95dccd4 100644 --- a/configure.ac +++ b/configure.ac @@ -676,6 +676,12 @@ GIT_CHECK_FUNC(mkdtemp, [NO_MKDTEMP=], [NO_MKDTEMP=YesPlease]) AC_SUBST(NO_MKDTEMP) +# Define NO_MKSTEMPS if you don't have mkstemps in the C library. +GIT_CHECK_FUNC(mkstemps, +[NO_MKSTEMPS=], +[NO_MKSTEMPS=YesPlease]) +AC_SUBST(NO_MKSTEMPS) +# # # Define NO_MMAP if you want to avoid mmap. # diff --git a/git-compat-util.h b/git-compat-util.h index c7cf2d5..f7217ad 100644 --- a/git-compat-util.h +++ b/git-compat-util.h @@ -232,6 +232,11 @@ extern int gitsetenv(const char *, const char *, int); extern char *gitmkdtemp(char *); #endif +#ifdef NO_MKSTEMPS +#define mkstemps gitmkstemps +extern int gitmkstemps(char *, int); +#endif + #ifdef NO_UNSETENV #define unsetenv gitunsetenv extern void gitunsetenv(const char *); -- 1.6.3.1.30.g55524 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF 2009-05-28 6:11 [PATCH v2 1/2] compat: add a mkstemps() compatibility function David Aguilar @ 2009-05-28 6:11 ` David Aguilar 2009-05-28 17:44 ` Jeff King 2009-05-28 7:07 ` [PATCH v2 1/2] compat: add a mkstemps() compatibility function Johannes Sixt 1 sibling, 1 reply; 7+ messages in thread From: David Aguilar @ 2009-05-28 6:11 UTC (permalink / raw) To: git; +Cc: gitster, markus.heidelberg, jnareb, j.sixt, David Aguilar Naturally, prep_temp_blob() did not care about filenames. As a result, scripts that use GIT_EXTERNAL_DIFF ended up with filenames such as ".diff_XXXXXX". This specializes the GIT_EXTERNAL_DIFF code to generate user-friendly filenames when creating temporary files. Diffing "name.ext" now generates "XXXXXX_name.ext". Signed-off-by: David Aguilar <davvid@gmail.com> --- This includes Hannes' suggestion to avoid basename() for Windows portability. cache.h | 2 ++ diff.c | 45 +++++++++++++++++++++++++++++++++++++-------- path.c | 19 +++++++++++++++++++ t/t4020-diff-external.sh | 9 +++++++++ 4 files changed, 67 insertions(+), 8 deletions(-) diff --git a/cache.h b/cache.h index b8503ad..871c984 100644 --- a/cache.h +++ b/cache.h @@ -614,6 +614,8 @@ extern int is_empty_blob_sha1(const unsigned char *sha1); int git_mkstemp(char *path, size_t n, const char *template); +int git_mkstemps(char *path, size_t n, const char *template, int suffix_len); + /* * NOTE NOTE NOTE!! * diff --git a/diff.c b/diff.c index dcfbcb0..60f07a0 100644 --- a/diff.c +++ b/diff.c @@ -1960,12 +1960,37 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp, void *blob, unsigned long size, const unsigned char *sha1, - int mode) + int mode, + int pretty_filename) { int fd; struct strbuf buf = STRBUF_INIT; - fd = git_mkstemp(temp->tmp_path, PATH_MAX, ".diff_XXXXXX"); + if (pretty_filename) { + /* Generate "XXXXXX_filename" */ + struct strbuf pretty_name = STRBUF_INIT; + char *basename = ((char*)path) + strlen(path) - 1; + + /* Windows lacks basename() */ + while(*basename && basename > path) { + basename--; + if (is_dir_sep(*basename)) { + basename++; + break; + } + } + + strbuf_addstr(&pretty_name, "XXXXXX_"); + strbuf_addstr(&pretty_name, basename); + + fd = git_mkstemps(temp->tmp_path, PATH_MAX, + pretty_name.buf, strlen(basename) + 1); + + strbuf_release(&pretty_name); + } + else { + fd = git_mkstemp(temp->tmp_path, PATH_MAX, ".diff_XXXXXX"); + } if (fd < 0) die("unable to create temp-file: %s", strerror(errno)); if (convert_to_working_tree(path, @@ -1984,7 +2009,8 @@ static void prep_temp_blob(const char *path, struct diff_tempfile *temp, } static struct diff_tempfile *prepare_temp_file(const char *name, - struct diff_filespec *one) + struct diff_filespec *one, + int pretty_filename) { struct diff_tempfile *temp = claim_diff_tempfile(); @@ -2021,7 +2047,8 @@ static struct diff_tempfile *prepare_temp_file(const char *name, (one->sha1_valid ? one->sha1 : null_sha1), (one->sha1_valid ? - one->mode : S_IFLNK)); + one->mode : S_IFLNK), + pretty_filename); strbuf_release(&sb); } else { @@ -2045,7 +2072,7 @@ static struct diff_tempfile *prepare_temp_file(const char *name, if (diff_populate_filespec(one, 0)) die("cannot read data blob for %s", one->path); prep_temp_blob(name, temp, one->data, one->size, - one->sha1, one->mode); + one->sha1, one->mode, pretty_filename); } return temp; } @@ -2071,8 +2098,9 @@ static void run_external_diff(const char *pgm, if (one && two) { struct diff_tempfile *temp_one, *temp_two; const char *othername = (other ? other : name); - temp_one = prepare_temp_file(name, one); - temp_two = prepare_temp_file(othername, two); + int pretty_filename = 1; + temp_one = prepare_temp_file(name, one, pretty_filename); + temp_two = prepare_temp_file(othername, two, pretty_filename); *arg++ = pgm; *arg++ = name; *arg++ = temp_one->name; @@ -3574,8 +3602,9 @@ static char *run_textconv(const char *pgm, struct diff_filespec *spec, const char **arg = argv; struct child_process child; struct strbuf buf = STRBUF_INIT; + int pretty_filename = 0; - temp = prepare_temp_file(spec->path, spec); + temp = prepare_temp_file(spec->path, spec, pretty_filename); *arg++ = pgm; *arg++ = temp->name; *arg = NULL; diff --git a/path.c b/path.c index 8a0a674..090b490 100644 --- a/path.c +++ b/path.c @@ -140,6 +140,25 @@ int git_mkstemp(char *path, size_t len, const char *template) } + +/* git_mkstemps() - create tmp file with suffix honoring TMPDIR variable. */ +int git_mkstemps(char *path, size_t len, const char *template, int suffix_len) +{ + const char *tmp; + size_t n; + + tmp = getenv("TMPDIR"); + if (!tmp) + tmp = "/tmp"; + n = snprintf(path, len, "%s/%s", tmp, template); + if (len <= n) { + errno = ENAMETOOLONG; + return -1; + } + return mkstemps(path, suffix_len); +} + + int validate_headref(const char *path) { struct stat st; diff --git a/t/t4020-diff-external.sh b/t/t4020-diff-external.sh index 0720001..4ea42e0 100755 --- a/t/t4020-diff-external.sh +++ b/t/t4020-diff-external.sh @@ -136,6 +136,15 @@ test_expect_success 'GIT_EXTERNAL_DIFF with more than one changed files' ' GIT_EXTERNAL_DIFF=echo git diff ' +test_expect_success 'GIT_EXTERNAL_DIFF generates pretty paths' ' + touch file.ext && + git add file.ext && + echo with extension > file.ext && + GIT_EXTERNAL_DIFF=echo git diff file.ext | grep ......_file\.ext && + git update-index --force-remove file.ext && + rm file.ext +' + echo "#!$SHELL_PATH" >fake-diff.sh cat >> fake-diff.sh <<\EOF cat $2 >> crlfed.txt -- 1.6.3.1.30.g55524 ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF 2009-05-28 6:11 ` [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF David Aguilar @ 2009-05-28 17:44 ` Jeff King 2009-05-28 21:30 ` David Aguilar 0 siblings, 1 reply; 7+ messages in thread From: Jeff King @ 2009-05-28 17:44 UTC (permalink / raw) To: David Aguilar; +Cc: git, gitster, markus.heidelberg, jnareb, j.sixt On Wed, May 27, 2009 at 11:11:17PM -0700, David Aguilar wrote: > +int git_mkstemps(char *path, size_t n, const char *template, int suffix_len); FWIW, I find this name not very descriptive. From the name, I would expect it to do the exact same thing as mkstemps, but be our own personal implementation. But it is actually a wrapper that behaves somewhat differently. So I wonder if "mkstemps_tmpdir" or something would be a better name. > + if (pretty_filename) { > + /* Generate "XXXXXX_filename" */ Is there a reason _not_ to always just use the pretty filename? It looks like you turn it on for external diff, but off for textconv. I don't think there is a reason not to use it for textconv. Is there some other code path that changes it that I'm missing? > + int pretty_filename = 1; > + temp_one = prepare_temp_file(name, one, pretty_filename); > + temp_two = prepare_temp_file(othername, two, pretty_filename); I think this reads much better as just: temp_one = prepare_temp_file(name, one, 1); temp_two = prepare_temp_file(othername, two, 1); Then it eliminates one bit of state for the reader to keep track of; I don't have to wonder if pretty_filename might ever change. If I care about what the '1' means, I can go look at the definition of prepare_temp_file (or if you really want to make it more self-documenting, make it a "flags" field and set the USE_PRETTY_FILENAME flag). However, I suspect that all callers should use pretty filenames, and then this bit would just go away. > + int pretty_filename = 0; > > - temp = prepare_temp_file(spec->path, spec); > + temp = prepare_temp_file(spec->path, spec, pretty_filename); Ditto here. -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF 2009-05-28 17:44 ` Jeff King @ 2009-05-28 21:30 ` David Aguilar 2009-05-28 22:06 ` Jeff King 0 siblings, 1 reply; 7+ messages in thread From: David Aguilar @ 2009-05-28 21:30 UTC (permalink / raw) To: Jeff King; +Cc: git, gitster, markus.heidelberg, jnareb, j.sixt On Thu, May 28, 2009 at 01:44:36PM -0400, Jeff King wrote: > On Wed, May 27, 2009 at 11:11:17PM -0700, David Aguilar wrote: > > > +int git_mkstemps(char *path, size_t n, const char *template, int suffix_len); > > FWIW, I find this name not very descriptive. From the name, I would > expect it to do the exact same thing as mkstemps, but be our own > personal implementation. But it is actually a wrapper that behaves > somewhat differently. So I wonder if "mkstemps_tmpdir" or something > would be a better name. It does exactly what git_mkstemp() does, plus the extra suffix_len parameter. If we rename this function we have to rename both. > Is there a reason _not_ to always just use the pretty filename? It looks > like you turn it on for external diff, but off for textconv. I don't > think there is a reason not to use it for textconv. I was not aware of the other code paths and only wanted to affect the one that I knew about. I agree that making that the default behavior would be great, meaning we could drop the pretty_filename flag altogether. If you and others agree that the user-friendly names are a good thing to have by default then I can rework patch 2/2. > However, I suspect that all callers should use pretty filenames, and > then this bit would just go away. I fully agree. -- David ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF 2009-05-28 21:30 ` David Aguilar @ 2009-05-28 22:06 ` Jeff King 0 siblings, 0 replies; 7+ messages in thread From: Jeff King @ 2009-05-28 22:06 UTC (permalink / raw) To: David Aguilar; +Cc: git, gitster, markus.heidelberg, jnareb, j.sixt On Thu, May 28, 2009 at 02:30:49PM -0700, David Aguilar wrote: > > FWIW, I find this name not very descriptive. From the name, I would > > expect it to do the exact same thing as mkstemps, but be our own > > personal implementation. But it is actually a wrapper that behaves > > somewhat differently. So I wonder if "mkstemps_tmpdir" or something > > would be a better name. > > It does exactly what git_mkstemp() does, plus the extra > suffix_len parameter. If we rename this function we have to > rename both. Ah. Well, I think that is a crappy name, too, then, but it is not your fault. ;) So if there is precedent, I guess somebody else thought it was a good idea, and you should leave your patch as-is. > I was not aware of the other code paths and only wanted to > affect the one that I knew about. I agree that making that the > default behavior would be great, meaning we could drop the > pretty_filename flag altogether. > > If you and others agree that the user-friendly names are a good > thing to have by default then I can rework patch 2/2. Switching to user-friendly temp filenames for textconv was on my todo list, so I definitely think it is a good idea. -Peff ^ permalink raw reply [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] compat: add a mkstemps() compatibility function 2009-05-28 6:11 [PATCH v2 1/2] compat: add a mkstemps() compatibility function David Aguilar 2009-05-28 6:11 ` [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF David Aguilar @ 2009-05-28 7:07 ` Johannes Sixt 2009-05-28 9:31 ` David Aguilar 1 sibling, 1 reply; 7+ messages in thread From: Johannes Sixt @ 2009-05-28 7:07 UTC (permalink / raw) To: David Aguilar; +Cc: git, gitster, markus.heidelberg, jnareb David Aguilar schrieb: > +++ 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). ^ permalink raw reply related [flat|nested] 7+ messages in thread
* Re: [PATCH v2 1/2] compat: add a mkstemps() compatibility function 2009-05-28 7:07 ` [PATCH v2 1/2] compat: add a mkstemps() compatibility function Johannes Sixt @ 2009-05-28 9:31 ` David Aguilar 0 siblings, 0 replies; 7+ messages in thread From: David Aguilar @ 2009-05-28 9:31 UTC (permalink / raw) To: Johannes Sixt; +Cc: git, gitster, markus.heidelberg, jnareb On Thu, May 28, 2009 at 09:07:32AM +0200, Johannes Sixt wrote: > David Aguilar schrieb: > > 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 Thanks. I'll squash these in and resend 1/2 in a moment. -- David ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2009-05-28 22:06 UTC | newest] Thread overview: 7+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2009-05-28 6:11 [PATCH v2 1/2] compat: add a mkstemps() compatibility function David Aguilar 2009-05-28 6:11 ` [PATCH v2 2/2] diff: generate prettier filenames when using GIT_EXTERNAL_DIFF David Aguilar 2009-05-28 17:44 ` Jeff King 2009-05-28 21:30 ` David Aguilar 2009-05-28 22:06 ` Jeff King 2009-05-28 7:07 ` [PATCH v2 1/2] compat: add a mkstemps() compatibility function Johannes Sixt 2009-05-28 9:31 ` David Aguilar
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).