{"thread":{"id":"61327","subject":"[PATCH] apply: avoid using fixed-size buffer in write_out_one_reject()","startedAt":"2024-04-16T20:16:07Z","lastAt":"2024-04-16T22:39:17Z","messageCount":2,"participants":["René Scharfe","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"493006","messageId":"a93cd243-cb17-4ad5-8d23-30768dc5213b@web.de","threadId":"61327","inReplyTo":null,"subject":"[PATCH] apply: avoid using fixed-size buffer in write_out_one_reject()","fromName":"René Scharfe","fromEmail":"l.s.r@web.de","sentAt":"2024-04-16T20:16:05Z","receivedAt":"2024-04-16T20:16:07Z","isPatch":true,"sender":{"key":"l.s.r@web.de","avatar":"https://avatars.githubusercontent.com/u/26122331?v=4"},"body":"On some systems PATH_MAX is not a hard limit.  Support longer paths by\nbuilding them on the heap instead of using static buffers.\n\nTake care to work around (arguably buggy) implementations of free(3)\nthat change errno by calling it only after using the errno value.\n\nSigned-off-by: René Scharfe <l.s.r@web.de>\n---\n apply.c | 33 +++++++++++++++++----------------\n 1 file changed, 17 insertions(+), 16 deletions(-)\n\ndiff --git a/apply.c b/apply.c\nindex 34f20326a7..757009a39e 100644\n--- a/apply.c\n+++ b/apply.c\n@@ -4612,7 +4612,7 @@ static int write_out_one_result(struct apply_state *state,\n static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n {\n \tFILE *rej;\n-\tchar namebuf[PATH_MAX];\n+\tchar *namebuf;\n \tstruct fragment *frag;\n \tint fd, cnt = 0;\n \tstruct strbuf sb = STRBUF_INIT;\n@@ -4645,30 +4645,29 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \t\tsay_patch_name(stderr, sb.buf, patch);\n \tstrbuf_release(&sb);\n\n-\tcnt = strlen(patch->new_name);\n-\tif (ARRAY_SIZE(namebuf) <= cnt + 5) {\n-\t\tcnt = ARRAY_SIZE(namebuf) - 5;\n-\t\twarning(_(\"truncating .rej filename to %.*s.rej\"),\n-\t\t\tcnt - 1, patch->new_name);\n-\t}\n-\tmemcpy(namebuf, patch->new_name, cnt);\n-\tmemcpy(namebuf + cnt, \".rej\", 5);\n+\tnamebuf = xstrfmt(\"%s.rej\", patch->new_name);\n\n \tfd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n \tif (fd < 0) {\n-\t\tif (errno != EEXIST)\n-\t\t\treturn error_errno(_(\"cannot open %s\"), namebuf);\n-\t\tif (unlink(namebuf))\n-\t\t\treturn error_errno(_(\"cannot unlink '%s'\"), namebuf);\n+\t\tif (errno != EEXIST) {\n+\t\t\terror_errno(_(\"cannot open %s\"), namebuf);\n+\t\t\tgoto error;\n+\t\t}\n+\t\tif (unlink(namebuf)) {\n+\t\t\terror_errno(_(\"cannot unlink '%s'\"), namebuf);\n+\t\t\tgoto error;\n+\t\t}\n \t\tfd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);\n-\t\tif (fd < 0)\n-\t\t\treturn error_errno(_(\"cannot open %s\"), namebuf);\n+\t\tif (fd < 0) {\n+\t\t\terror_errno(_(\"cannot open %s\"), namebuf);\n+\t\t\tgoto error;\n+\t\t}\n \t}\n \trej = fdopen(fd, \"w\");\n \tif (!rej) {\n \t\terror_errno(_(\"cannot open %s\"), namebuf);\n \t\tclose(fd);\n-\t\treturn -1;\n+\t\tgoto error;\n \t}\n\n \t/* Normal git tools never deal with .rej, so do not pretend\n@@ -4693,6 +4692,8 @@ static int write_out_one_reject(struct apply_state *state, struct patch *patch)\n \t\t\tfputc('\\n', rej);\n \t}\n \tfclose(rej);\n+error:\n+\tfree(namebuf);\n \treturn -1;\n }\n\n--\n2.44.0\n"},{"id":"493011","messageId":"xmqqr0f5c4q5.fsf@gitster.g","threadId":"61327","inReplyTo":"a93cd243-cb17-4ad5-8d23-30768dc5213b@web.de","subject":"Re: [PATCH] apply: avoid using fixed-size buffer in write_out_one_reject()","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2024-04-16T22:39:14Z","receivedAt":"2024-04-16T22:39:17Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"René Scharfe <l.s.r@web.de> writes:\n\n> Take care to work around (arguably buggy) implementations of free(3)\n> that change errno by calling it only after using the errno value.\n\n;-).\n\nThe \"do everything including giving an error message, jump to the\nexit label where resources are released just before returning\"\npattern naturally gives us that, which is very nice.\n\nWill queue.  Thanks.\n"}]}