Re: [PATCH] Fix git_mkstemp to return an error when path is too long.
From: Junio C Hamano <hidden>
Date: 2016-06-15 22:43:24
Subsystem:
the rest · Maintainer:
Linus Torvalds
Possibly related (same subject, not in this thread)
- 2016-06-15 · [PATCH] Fix git_mkstemp to return an error when path is too long. · Carlos Rica <hidden>
- 2016-06-15 · Re: [PATCH] Fix git_mkstemp to return an error when path is too long. · Johannes Schindelin <hidden>
Carlos Rica [off-list ref] writes:
Now the function returns -2 to the caller if the given buffer is too short to save the entire path for the temporary file. Signed-off-by: Carlos Rica <redacted>
Trying not to overstep the static buffer is of course a good thing, and I think it is probably Ok to error out on a TMPDIR environment variable that is insanely long, instead of using an extra malloc/free, as underlying mkstemp() would error out if it is given an insanely long template buffer anyway. However, I think it is not so useful to return -2, even if that is done so that you can differentiate the case where the TMPDIR and/or the template were too long and the case mkstemp() errored out. Stop and think for a minute: what does the underlying mkstemp() do, if the given template is too long? That's right. You would get ENAMETOOLONG. So why don't we do this instead? --- diff.c | 2 +- path.c | 25 ++++++++++--------------- 2 files changed, 11 insertions(+), 16 deletions(-)
diff --git a/diff.c b/diff.c
index cd6b0c4..a5fc56b 100644
--- a/diff.c
+++ b/diff.c@@ -1695,7 +1695,7 @@ static void prep_temp_blob(struct diff_tempfile *temp, fd = git_mkstemp(temp->tmp_path, PATH_MAX, ".diff_XXXXXX"); if (fd < 0) - die("unable to create temp-file"); + die("unable to create temp-file: %s", strerror(errno)); if (write_in_full(fd, blob, size) != size) die("unable to write temp-file"); close(fd);
diff --git a/path.c b/path.c
index c4ce962..dc7ded9 100644
--- a/path.c
+++ b/path.c@@ -71,25 +71,20 @@ char *git_path(const char *fmt, ...) /* git_mkstemp() - create tmp file honoring TMPDIR variable */ int git_mkstemp(char *path, size_t len, const char *template) { - char *env, *pch = path; - - if ((env = getenv("TMPDIR")) == NULL) { - strcpy(pch, "/tmp/"); - len -= 5; - pch += 5; - } else { - size_t n = snprintf(pch, len, "%s/", env); - - len -= n; - pch += n; + 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; } - - strlcpy(pch, template, len); - return mkstemp(path); } - int validate_headref(const char *path) { struct stat st;