Download raw body.
improve 'got patch' error message upon malformed patch
On 07/12/24 20:19, Stefan Sperling wrote:
> 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.)
Yay, looks good to me; it's a very nice improvement.
>
> 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 \
>
improve 'got patch' error message upon malformed patch