"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
improve 'got patch' error message upon malformed patch
To:
gameoftrees@openbsd.org
Date:
Sat, 7 Dec 2024 20:19:23 +0100

Download raw body.

Thread 
When a patch is malformed, got patch just prints "malformed patch"
without any indication regarding the root cause.

For instance, trying to apply Kyle's memleak regress diff:

  got: malformed patch

The diff below makes 'got patch' show the line it was processing: 

  $ got patch /tmp/memleak.patch
  got-read-patch: @@ -287,6 +293,13 @@ test_done(): malformed patch
  G  regress/cmdline/add.sh
  got: malformed patch
  $  

(For whatever reason, the + length shown in this hunk header of Kyle's
patch is 13, while the hunk only contains 12 lines. Changing this 13
into 12 made the patch apply.)


M  libexec/got-read-patch/got-read-patch.c  |  24+  16-
M  regress/cmdline/patch.sh                 |  17+   2-

2 files changed, 41 insertions(+), 18 deletions(-)

commit - 29aa9c8fcf7bb6e251205fa7d4861666c0a9ff76
commit + a16c08c02d896f7eb4661d7b3d599f9b2c65de25
blob - fc59997853341c35bfc256bfabc86d7d3a5ff89f
blob + ed35f4ea7ec319548a7a11e1a9658f9a156b48e6
--- libexec/got-read-patch/got-read-patch.c
+++ libexec/got-read-patch/got-read-patch.c
@@ -334,7 +334,8 @@ find_diff(int *done, int *next, FILE *fp, int git, con
 			create = !strncmp(line+4, "0,0", 3);
 			if ((old == NULL && new == NULL) ||
 			    (!create && old == NULL))
-				err = got_error(GOT_ERR_PATCH_MALFORMED);
+				err = got_error_fmt(GOT_ERR_PATCH_MALFORMED,
+				    "%s", line);
 			else
 				err = send_patch(old, new, commitid,
 				    blob, xbit, git);
@@ -374,7 +375,8 @@ strtolnum(char **str, int *n)
 
 	*n = strtonum(*str, 0, INT_MAX, &errstr);
 	if (errstr != NULL)
-		return got_error(GOT_ERR_PATCH_MALFORMED);
+		return got_error_fmt(GOT_ERR_PATCH_MALFORMED,
+		    "%s: %s", *str, errstr);
 
 	*p = c;
 	*str = p;
@@ -384,7 +386,8 @@ strtolnum(char **str, int *n)
 static const struct got_error *
 parse_hdr(char *s, int *done, struct got_imsg_patch_hunk *hdr)
 {
-	static const struct got_error *err = NULL;
+	const struct got_error *err = NULL;
+	char *s0 = s; 
 
 	if (strncmp(s, "@@ -", 4)) {
 		*done = 1;
@@ -409,7 +412,7 @@ parse_hdr(char *s, int *done, struct got_imsg_patch_hu
 		s++;
 
 	if (*s != '+' || !*++s)
-		return got_error(GOT_ERR_PATCH_MALFORMED);
+		return got_error_fmt(GOT_ERR_PATCH_MALFORMED, "%s", s0);
 	err = strtolnum(&s, &hdr->newfrom);
 	if (err)
 		return err;
@@ -425,14 +428,14 @@ parse_hdr(char *s, int *done, struct got_imsg_patch_hu
 		s++;
 
 	if (*s != '@')
-		return got_error(GOT_ERR_PATCH_MALFORMED);
+		return got_error_fmt(GOT_ERR_PATCH_MALFORMED, "%s", s0);
 
 	if (hdr->oldfrom >= INT_MAX - hdr->oldlines ||
 	    hdr->newfrom >= INT_MAX - hdr->newlines ||
 	    /* not so sure about this one */
 	    hdr->oldlines >= INT_MAX - hdr->newlines - 1 ||
 	    (hdr->oldlines == 0 && hdr->newlines == 0))
-		return got_error(GOT_ERR_PATCH_MALFORMED);
+		return got_error_fmt(GOT_ERR_PATCH_MALFORMED, "%s", s0);
 
 	if (hdr->oldlines == 0) {
 		/* larry says to "do append rather than insert"; I don't
@@ -451,7 +454,7 @@ parse_hdr(char *s, int *done, struct got_imsg_patch_hu
 static const struct got_error *
 send_line(const char *line, size_t len)
 {
-	static const struct got_error *err = NULL;
+	const struct got_error *err = NULL;
 	struct iovec iov[2];
 	int iovcnt = 0;
 
@@ -504,7 +507,7 @@ peek_special_line(FILE *fp)
 static const struct got_error *
 parse_hunk(FILE *fp, int *done)
 {
-	static const struct got_error *err = NULL;
+	const struct got_error *err = NULL;
 	struct got_imsg_patch_hunk hdr;
 	char	*line = NULL, ch;
 	size_t	 linesize = 0;
@@ -516,6 +519,8 @@ parse_hunk(FILE *fp, int *done)
 		*done = 1;
 		goto done;
 	}
+	if (line[linelen - 1] == '\n')
+		line[linelen - 1] = '\0';
 
 	err = parse_hdr(line, done, &hdr);
 	if (err)
@@ -569,12 +574,14 @@ parse_hunk(FILE *fp, int *done)
 			leftnew--;
 			break;
 		default:
-			err = got_error(GOT_ERR_PATCH_MALFORMED);
+			err = got_error_fmt(GOT_ERR_PATCH_MALFORMED,
+			    "%s", line);
 			goto done;
 		}
 
 		if (leftold < 0 || leftnew < 0) {
-			err = got_error(GOT_ERR_PATCH_MALFORMED);
+			err = got_error_fmt(GOT_ERR_PATCH_MALFORMED,
+			    "%s", line);
 			goto done;
 		}
 
@@ -692,17 +699,18 @@ main(int argc, char **argv)
 	err = got_privsep_flush_imsg(&ibuf);
 	imsg_free(&imsg);
 done:
+	if (err != NULL) {
+		if (err->code != GOT_ERR_PRIVSEP_PIPE) {
+			fprintf(stderr, "%s: %s\n", getprogname(), err->msg);
+			fflush(stderr);
+		}
+		got_privsep_send_error(&ibuf, err);
+	}
 	if (fd != -1 && close(fd) == -1 && err == NULL)
 		err = got_error_from_errno("close");
 	if (fp != NULL && fclose(fp) == EOF && err == NULL)
 		err = got_error_from_errno("fclose");
-	if (err != NULL) {
-		got_privsep_send_error(&ibuf, err);
-		err = NULL;
-	}
 	if (close(GOT_IMSG_FD_CHILD) == -1 && err == NULL)
 		err = got_error_from_errno("close");
-	if (err && err->code != GOT_ERR_PRIVSEP_PIPE)
-		fprintf(stderr, "%s: %s\n", getprogname(), err->msg);
 	return err ? 1 : 0;
 }
blob - 66b8ac0292d449f916eeb70817e844ea1710dd4b
blob + e5504f42e6a08f89598536b78ff4c5ed56bfb461
--- regress/cmdline/patch.sh
+++ regress/cmdline/patch.sh
@@ -233,7 +233,9 @@ test_patch_malformed() {
 EOF
 
 	echo -n > $testroot/stdout.expected
-	echo "got: malformed patch" > $testroot/stderr.expected
+	echo "got-read-patch: @@ -1 +1,2: malformed patch" \
+		> $testroot/stderr.expected
+	echo "got: malformed patch" >> $testroot/stderr.expected
 
 	(cd $testroot/wt && got patch patch) \
 		 > $testroot/stdout \
@@ -253,6 +255,10 @@ EOF
 		return 1
 	fi
 
+	echo -n > $testroot/stdout.expected
+	echo "got-read-patch: @@ -1 +1,2: malformed patch" \
+		> $testroot/stderr.expected
+	echo "got: malformed patch" >> $testroot/stderr.expected
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -288,6 +294,10 @@ EOF
 		return 1
 	fi
 
+	echo "got-read-patch: alpha: malformed patch" \
+		> $testroot/stderr.expected
+	echo "got: malformed patch" >> $testroot/stderr.expected
+
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -322,6 +332,10 @@ EOF
 		return 1
 	fi
 
+	echo "got-read-patch: @@ -0,0 +0,0 @@: malformed patch" \
+		> $testroot/stderr.expected
+	echo "got: malformed patch" >> $testroot/stderr.expected
+
 	cmp -s $testroot/stderr.expected $testroot/stderr
 	ret=$?
 	if [ $ret -ne 0 ]; then
@@ -354,7 +368,8 @@ there's no patch in here!
 EOF
 
 	echo -n > $testroot/stdout.expected
-	echo "got: no patch found" > $testroot/stderr.expected
+	echo "got-read-patch: no patch found" > $testroot/stderr.expected
+	echo "got: no patch found" >> $testroot/stderr.expected
 
 	(cd $testroot/wt && got patch patch) \
 		 > $testroot/stdout \