Re: "git am" crash (builtin/apply.c:2108) + small repro
Thanks for looking into this, guys!
I seem to run into this with some regularity, but my setting is apply.whitespace=strip rather than 'fix'.
Is there an obvious workaround?
Here are my remaining settings, sanitized for file paths and URLs:
svn.rmdir=true push.default=upstream color.ui=auto color.diff=auto color.status=auto color.branch=auto color.interactive=auto color.branch.current=white blue bold color.branch.local=blue color.branch.remote=green color.diff.plain=white color.diff.meta=yellow bold color.diff.frag=magenta bold color.diff.old=red color.diff.new=green color.diff.whitespace=red blink color.status.added=yellow color.status.changed=green color.status.untracked=cyan svn.followparent=true log.date=relative blame.date=short diff.renames=copies diff.copies=true diff.mnemonicprefix=true apply.whitespace=strip merge.tool=emerge status.relativepaths=true web.browser=lynx rebase.stat=true core.repositoryformatversion=0 core.filemode=true core.bare=false core.logallrefupdates=true rerere.enabled=true branch.autosetuprebase=always
-a
On Fri, Oct 12, 2012 at 3:53 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 112 quoted lines
> Junio C Hamano <gitster@pobox.com> writes:
>
>> I've never been a fan of update_pre_post_images(). I think I know
>> what is going on (--whitespace=fix). Will post a fix sometime
>> later.
>
> It bisects down to 5166714 (apply: Allow blank context lines to
> match beyond EOF, 2010-03-06).
>
> I do not like this patch at all (I _think_ it should somehow check
> the corresponding postimage is sane when the "ah, we ran out of
> preimage because the caller fixed it to reduce trailing blank lines"
> case), but at least this should get the ball rolling.
>
> Björn, both of the changes involved are yours, so I am thinking that
> you may be a good person to give a sanity check on this. No need to
> hurry, as this is not a recent regression and we are in a pre-release
> freeze.
>
> Thanks.
>
> -- >8 --
> Subject: apply.c:update_pre_post_images(): the preimage can be truncated
>
> 5166714 (apply: Allow blank context lines to match beyond EOF,
> 2010-03-06) and then later 0c3ef98 (apply: Allow blank *trailing*
> context lines to match beyond EOF, 2010-04-08) taught "git apply"
> to trim new blank lines at the end in the patch text when matching
> the contents being patched and the preimage recorded in the patch,
> under --whitespace=fix mode.
>
> When a preimage is modified to match the current contents in
> preparation for such a "fixed" patch application, the context lines
> in the postimage must be updated to match (otherwise, it would
> reintroduce whitespace breakages), and update_pre_post_images()
> function is responsible for doing this. However, this function was
> not updated to take into account a case where the removal of
> trailing blank lines reduces the number of lines in the preimage,
> and triggered an assertion error.
>
> The logic to fix the postimage by copying the corrected context
> lines from the preimage was not prepared to handle this case,
> either, but it was protected by the assert() and only got exposed
> when the assertion is corrected.
>
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
> builtin/apply.c | 23 +++++++++++++++++------
> 1 file changed, 17 insertions(+), 6 deletions(-)
>
> diff --git c/builtin/apply.c w/builtin/apply.c
> index 156b3ce..6c11e8b 100644
> --- c/builtin/apply.c
> +++ w/builtin/apply.c
> @@ -2095,7 +2095,7 @@ static void update_pre_post_images(struct image *preimage,
> char *buf,
> size_t len, size_t postlen)
> {
> - int i, ctx;
> + int i, ctx, reduced;
> char *new, *old, *fixed;
> struct image fixed_preimage;
>
> @@ -2105,8 +2105,10 @@ static void update_pre_post_images(struct image *preimage,
> * free "oldlines".
> */
> prepare_image(&fixed_preimage, buf, len, 1);
> - assert(fixed_preimage.nr == preimage->nr);
> - for (i = 0; i < preimage->nr; i++)
> + assert(postlen
> + ? fixed_preimage.nr == preimage->nr
> + : fixed_preimage.nr <= preimage->nr);
> + for (i = 0; i < fixed_preimage.nr; i++)
> fixed_preimage.line[i].flag = preimage->line[i].flag;
> free(preimage->line_allocated);
> *preimage = fixed_preimage;
> @@ -2126,7 +2128,8 @@ static void update_pre_post_images(struct image *preimage,
> else
> new = old;
> fixed = preimage->buf;
> - for (i = ctx = 0; i < postimage->nr; i++) {
> +
> + for (i = reduced = ctx = 0; i < postimage->nr; i++) {
> size_t len = postimage->line[i].len;
> if (!(postimage->line[i].flag & LINE_COMMON)) {
> /* an added line -- no counterparts in preimage */
> @@ -2145,8 +2148,15 @@ static void update_pre_post_images(struct image *preimage,
> fixed += preimage->line[ctx].len;
> ctx++;
> }
> - if (preimage->nr <= ctx)
> - die(_("oops"));
> +
> + /*
> + * preimage is expected to run out, if the caller
> + * fixed addition of trailing blank lines.
> + */
> + if (preimage->nr <= ctx) {
> + reduced++;
> + continue;
> + }
>
> /* and copy it in, while fixing the line length */
> len = preimage->line[ctx].len;
> @@ -2159,6 +2169,7 @@ static void update_pre_post_images(struct image *preimage,
>
> /* Fix the length of the whole thing */
> postimage->len = new - postimage->buf;
> + postimage->nr -= reduced;
> }
>
> static int match_fragment(struct image *img,
