apply: avoid using fixed-size buffer in write_out_one_reject()

Commit Message

René Scharfe April 16, 2024, 8:16 p.m. UTC

On some systems PATH_MAX is not a hard limit.  Support longer paths by
building them on the heap instead of using static buffers.

Take care to work around (arguably buggy) implementations of free(3)
that change errno by calling it only after using the errno value.

Signed-off-by: René Scharfe <[e-mail]>
---
 apply.c | 33 +++++++++++++++++----------------
 1 file changed, 17 insertions(+), 16 deletions(-)

--
2.44.0

Comments

Junio C Hamano April 16, 2024, 10:39 p.m. UTC | #1

René Scharfe <[e-mail]> writes:

> Take care to work around (arguably buggy) implementations of free(3)
> that change errno by calling it only after using the errno value.

;-).

The "do everything including giving an error message, jump to the
exit label where resources are released just before returning"
pattern naturally gives us that, which is very nice.

Will queue.  Thanks.

Patch

diff --git a/apply.c b/apply.c
index 34f20326a7..757009a39e 100644
--- a/apply.c
+++ b/apply.c
@@ -4612,7 +4612,7 @@  static int write_out_one_result(struct apply_state *state,
 static int write_out_one_reject(struct apply_state *state, struct patch *patch)
 {
 	FILE *rej;
-	char namebuf[PATH_MAX];
+	char *namebuf;
 	struct fragment *frag;
 	int fd, cnt = 0;
 	struct strbuf sb = STRBUF_INIT;
@@ -4645,30 +4645,29 @@  static int write_out_one_reject(struct apply_state *state, struct patch *patch)
 		say_patch_name(stderr, sb.buf, patch);
 	strbuf_release(&sb);

-	cnt = strlen(patch->new_name);
-	if (ARRAY_SIZE(namebuf) <= cnt + 5) {
-		cnt = ARRAY_SIZE(namebuf) - 5;
-		warning(_("truncating .rej filename to %.*s.rej"),
-			cnt - 1, patch->new_name);
-	}
-	memcpy(namebuf, patch->new_name, cnt);
-	memcpy(namebuf + cnt, ".rej", 5);
+	namebuf = xstrfmt("%s.rej", patch->new_name);

 	fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
 	if (fd < 0) {
-		if (errno != EEXIST)
-			return error_errno(_("cannot open %s"), namebuf);
-		if (unlink(namebuf))
-			return error_errno(_("cannot unlink '%s'"), namebuf);
+		if (errno != EEXIST) {
+			error_errno(_("cannot open %s"), namebuf);
+			goto error;
+		}
+		if (unlink(namebuf)) {
+			error_errno(_("cannot unlink '%s'"), namebuf);
+			goto error;
+		}
 		fd = open(namebuf, O_CREAT | O_EXCL | O_WRONLY, 0666);
-		if (fd < 0)
-			return error_errno(_("cannot open %s"), namebuf);
+		if (fd < 0) {
+			error_errno(_("cannot open %s"), namebuf);
+			goto error;
+		}
 	}
 	rej = fdopen(fd, "w");
 	if (!rej) {
 		error_errno(_("cannot open %s"), namebuf);
 		close(fd);
-		return -1;
+		goto error;
 	}

 	/* Normal git tools never deal with .rej, so do not pretend
@@ -4693,6 +4692,8 @@  static int write_out_one_reject(struct apply_state *state, struct patch *patch)
 			fputc('\n', rej);
 	}
 	fclose(rej);
+error:
+	free(namebuf);
 	return -1;
 }