From: Karthik Nayak Date: Tue, 17 Mar 2026 09:40:57 GMT Subject: Re: [GSoC PATCH] apply: report input location in binary and garbage patch errors Message-ID: In-Reply-To: <20260317002235.6121-1-jerrywang183@yahoo.com> Jialong Wang writes: > Several binary parsing paths in apply.c still report only line > numbers. When more than one patch input is fed to a single > invocation, that does not tell the user which input the line belongs > to. > > Report the patch input location for corrupt and unrecognized binary > patches, as well as the "patch with only garbage" case, and update > the related tests. > > Signed-off-by: Jialong Wang > --- > apply.c | 10 ++++++---- > t/t4100-apply-stat.sh | 12 ++++++++++++ > t/t4103-apply-binary.sh | 20 +++++++++++++++++++- > 3 files changed, 37 insertions(+), 5 deletions(-) > > diff --git a/apply.c b/apply.c > index 700809f3e6..84b4a569c5 100644 > --- a/apply.c > +++ b/apply.c > @@ -2110,8 +2110,8 @@ static struct fragment *parse_binary_hunk(struct apply_state *state, > corrupt: > free(data); > *status_p = -1; > - error(_("corrupt binary patch at line %d: %.*s"), > - state->linenr-1, llen-1, buffer); > + error(_("corrupt binary patch at %s:%d: %.*s"), > + state->patch_input_file, state->linenr-1, llen-1, buffer); > return NULL; > } > > @@ -2147,7 +2147,8 @@ static int parse_binary(struct apply_state *state, > forward = parse_binary_hunk(state, &buffer, &size, &status, &used); > if (!forward && !status) > /* there has to be one hunk (forward hunk) */ > - return error(_("unrecognized binary patch at line %d"), state->linenr-1); > + return error(_("unrecognized binary patch at %s:%d"), > + state->patch_input_file, state->linenr-1); > if (status) > /* otherwise we already gave an error message */ > return status; > @@ -2309,7 +2310,8 @@ static int parse_chunk(struct apply_state *state, char *buffer, unsigned long si > */ > if ((state->apply || state->check) && > (!patch->is_binary && !metadata_changes(patch))) { > - error(_("patch with only garbage at line %d"), state->linenr); > + error(_("patch with only garbage at %s:%d"), > + state->patch_input_file, state->linenr); > return -128; > } > } > diff --git a/t/t4100-apply-stat.sh b/t/t4100-apply-stat.sh > index b3d93d8ed6..8393076469 100755 > --- a/t/t4100-apply-stat.sh > +++ b/t/t4100-apply-stat.sh > @@ -125,4 +125,16 @@ test_expect_success 'applying a patch with an invalid mode reports the input' ' > EOF > test_cmp expect err > ' > + > +test_expect_success 'applying a patch with only garbage reports the input' ' > + cat >garbage.patch <<-\EOF && > + diff --git a/f b/f > + --- a/f > + +++ b/f > + this is garbage > + EOF > + test_must_fail git apply garbage.patch 2>err && > + echo "error: patch with only garbage at garbage.patch:4" >expect && > + test_cmp expect err > +' > test_done > diff --git a/t/t4103-apply-binary.sh b/t/t4103-apply-binary.sh > index 8e302a5a57..f2d41e06bc 100755 > --- a/t/t4103-apply-binary.sh > +++ b/t/t4103-apply-binary.sh > @@ -179,6 +179,24 @@ test_expect_success PERL_TEST_HELPERS 'reject truncated binary diff' ' > " patch.trunc && > > do_reset && > - test_must_fail git apply patch.trunc > + test_must_fail git apply patch.trunc 2>err && > + line=$(awk "END { print NR + 1 }" patch.trunc) && > + grep "error: corrupt binary patch at patch.trunc:$line: " err > +' > + > +test_expect_success 'reject unrecognized binary diff' ' > + cat >patch.bad <<-\EOF && > + diff --git a/f b/f > + new file mode 100644 > + index 0000000..7898192 > + GIT binary patch > + bogus > + EOF > + test_must_fail git apply patch.bad 2>err && > + cat >expect <<-\EOF && > + error: unrecognized binary patch at patch.bad:4 > + error: No valid patches in input (allow with "--allow-empty") > + EOF > + test_cmp expect err > ' > test_done > -- > 2.51.0 This patch looks good to me! Thanks for picking this up. - Karthik