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