{"thread":{"id":"9227","subject":"[PATCH] Fix git_mkstemp to return an error when path is too long.","startedAt":"2007-07-26T01:32:52Z","lastAt":"2007-07-26T19:42:24Z","messageCount":4,"participants":["Carlos Rica","Johannes Schindelin","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"48617","messageId":"46A7F9C4.7030004@gmail.com","threadId":"9227","inReplyTo":null,"subject":"[PATCH] Fix git_mkstemp to return an error when path is too long.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-26T01:32:52Z","receivedAt":"2007-07-26T01:32:52Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"Now the function returns -2 to the caller if the given buffer\nis too short to save the entire path for the temporary file.\n\nSigned-off-by: Carlos Rica <jasampler@gmail.com>\n---\n diff.c |    2 ++\n path.c |    9 ++++++---\n 2 files changed, 8 insertions(+), 3 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex cd6b0c4..8735467 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1694,6 +1694,8 @@ static void prep_temp_blob(struct diff_tempfile *temp,\n \tint fd;\n\n \tfd = git_mkstemp(temp->tmp_path, PATH_MAX, \".diff_XXXXXX\");\n+\tif (fd == -2)\n+\t\tdie(\"path too long for temp-file\");\n \tif (fd < 0)\n \t\tdie(\"unable to create temp-file\");\n \tif (write_in_full(fd, blob, size) != size)\ndiff --git a/path.c b/path.c\nindex c4ce962..f33d15d 100644\n--- a/path.c\n+++ b/path.c\n@@ -68,7 +68,8 @@ char *git_path(const char *fmt, ...)\n }\n\n\n-/* git_mkstemp() - create tmp file honoring TMPDIR variable */\n+/* git_mkstemp() - create tmp file honoring TMPDIR variable.\n+ * return -2 if path is too long to have it concatenated. */\n int git_mkstemp(char *path, size_t len, const char *template)\n {\n \tchar *env, *pch = path;\n@@ -79,12 +80,14 @@ int git_mkstemp(char *path, size_t len, const char *template)\n \t\tpch += 5;\n \t} else {\n \t\tsize_t n = snprintf(pch, len, \"%s/\", env);\n-\n+\t\tif (n >= len)\n+\t\t\treturn -2;\n \t\tlen -= n;\n \t\tpch += n;\n \t}\n\n-\tstrlcpy(pch, template, len);\n+\tif (strlcpy(pch, template, len) >= len)\n+\t\treturn -2;\n\n \treturn mkstemp(path);\n }\n-- \n1.5.0\n"},{"id":"48629","messageId":"Pine.LNX.4.64.0707260433280.14781@racer.site","threadId":"9227","inReplyTo":"46A7F9C4.7030004@gmail.com","subject":"Re: [PATCH] Fix git_mkstemp to return an error when path is too long.","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-07-26T03:36:27Z","receivedAt":"2007-07-26T03:36:27Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Thu, 26 Jul 2007, Carlos Rica wrote:\n\n> @@ -79,12 +80,14 @@ int git_mkstemp(char *path, size_t len, const char *template)\n>  \t\tpch += 5;\n>  \t} else {\n>  \t\tsize_t n = snprintf(pch, len, \"%s/\", env);\n> -\n> +\t\tif (n >= len)\n> +\t\t\treturn -2;\n\nThat is certainly a bug fixed (even if few people have an insanely long \nTMPDIR...)\n\n>  \t\tlen -= n;\n>  \t\tpch += n;\n>  \t}\n> \n> -\tstrlcpy(pch, template, len);\n> +\tif (strlcpy(pch, template, len) >= len)\n> +\t\treturn -2;\n\nMaybe just \"return error(\"filename too long: %.*s\", 60, pch);\"?  So that \nall callers to git_mkstemp() get the message for free?\n\n\nCiao,\nDscho\n"},{"id":"48633","messageId":"7vabtjbxip.fsf@assigned-by-dhcp.cox.net","threadId":"9227","inReplyTo":"46A7F9C4.7030004@gmail.com","subject":"Re: [PATCH] Fix git_mkstemp to return an error when path is too long.","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2007-07-26T04:26:54Z","receivedAt":"2007-07-26T04:26:54Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Carlos Rica <jasampler@gmail.com> writes:\n\n> Now the function returns -2 to the caller if the given buffer\n> is too short to save the entire path for the temporary file.\n>\n> Signed-off-by: Carlos Rica <jasampler@gmail.com>\n\nTrying not to overstep the static buffer is of course a good\nthing, and I think it is probably Ok to error out on a TMPDIR\nenvironment variable that is insanely long, instead of using an\nextra malloc/free, as underlying mkstemp() would error out if it\nis given an insanely long template buffer anyway.\n\nHowever, I think it is not so useful to return -2, even if that\nis done so that you can differentiate the case where the TMPDIR\nand/or the template were too long and the case mkstemp() errored\nout.\n\nStop and think for a minute: what does the underlying mkstemp()\ndo, if the given template is too long?\n\nThat's right.  You would get ENAMETOOLONG.  So why don't we do\nthis instead?\n\n---\n diff.c |    2 +-\n path.c |   25 ++++++++++---------------\n 2 files changed, 11 insertions(+), 16 deletions(-)\n\ndiff --git a/diff.c b/diff.c\nindex cd6b0c4..a5fc56b 100644\n--- a/diff.c\n+++ b/diff.c\n@@ -1695,7 +1695,7 @@ static void prep_temp_blob(struct diff_tempfile *temp,\n \n \tfd = git_mkstemp(temp->tmp_path, PATH_MAX, \".diff_XXXXXX\");\n \tif (fd < 0)\n-\t\tdie(\"unable to create temp-file\");\n+\t\tdie(\"unable to create temp-file: %s\", strerror(errno));\n \tif (write_in_full(fd, blob, size) != size)\n \t\tdie(\"unable to write temp-file\");\n \tclose(fd);\ndiff --git a/path.c b/path.c\nindex c4ce962..dc7ded9 100644\n--- a/path.c\n+++ b/path.c\n@@ -71,25 +71,20 @@ char *git_path(const char *fmt, ...)\n /* git_mkstemp() - create tmp file honoring TMPDIR variable */\n int git_mkstemp(char *path, size_t len, const char *template)\n {\n-\tchar *env, *pch = path;\n-\n-\tif ((env = getenv(\"TMPDIR\")) == NULL) {\n-\t\tstrcpy(pch, \"/tmp/\");\n-\t\tlen -= 5;\n-\t\tpch += 5;\n-\t} else {\n-\t\tsize_t n = snprintf(pch, len, \"%s/\", env);\n-\n-\t\tlen -= n;\n-\t\tpch += n;\n+\tconst char *tmp;\n+\tsize_t n;\n+\n+\ttmp = getenv(\"TMPDIR\");\n+\tif (!tmp)\n+\t\ttmp = \"/tmp\";\n+\tn = snprintf(path, len, \"%s/%s\", tmp, template);\n+\tif (len <= n) {\n+\t\terrno = ENAMETOOLONG;\n+\t\treturn -1;\n \t}\n-\n-\tstrlcpy(pch, template, len);\n-\n \treturn mkstemp(path);\n }\n \n-\n int validate_headref(const char *path)\n {\n \tstruct stat st;\n"},{"id":"48734","messageId":"1b46aba20707261242j722ab827w6ff17ccbca47737d@mail.gmail.com","threadId":"9227","inReplyTo":"7vabtjbxip.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] Fix git_mkstemp to return an error when path is too long.","fromName":"Carlos Rica","fromEmail":"jasampler@gmail.com","sentAt":"2007-07-26T19:42:24Z","receivedAt":"2007-07-26T19:42:24Z","isPatch":true,"sender":{"key":"jasampler@gmail.com","avatar":null},"body":"I like that new solution. ENAMETOOLONG is fantastic!\n\nUsually, the function is called using the long PATH_MAX constant,\nhowever, the function doesn't force you to supply an insanely long\nbuffer like that, and therefore is nice to say when the limit is exceeded.\n"}]}