Re: Memory corruption when rebasing with git version 1.8.1.5 on arch

[Jeff King <peff@peff.net>]

On Fri, Mar 22, 2013 at 11:08:59AM -0700, Junio C Hamano wrote:

> This may be sufficient.  In the olden days, we relied on that all
> whitespace fixing rules made the result shorter and took advantage
> of it in update-pre-post-images to rewrite the images in place.  The
> oddball tab-in-indent (aka Python), however, can grow the result by
> expanding tabs (deemed "incorrect") in the input into runs of spaces
> (deemed "kosher").
> 
> Fortunately, we already support its more generalized form "match
> while ignoring whitespace differences" that can lengthen the result;
> as long as we correctly count the number of bytes needed to hold
> rewritten postimage, the existing logic in update_pre_post_images
> should be able to do the rest for us.

Yeah, your patch looks right to me. I do wonder if the optimization
here:

> @@ -2399,8 +2401,10 @@ static int match_fragment(struct image *img,
>  	 * hunk match.  Update the context lines in the postimage.
>  	 */
>  	fixed_buf = strbuf_detach(&fixed, &fixed_len);
> +	if (postlen < postimage->len)
> +		postlen = 0;
>  	update_pre_post_images(preimage, postimage,
> -			       fixed_buf, fixed_len, 0);
> +			       fixed_buf, fixed_len, postlen);

should simply go into update_pre_post_images (i.e., let it decide on
whether to do it inline or with a new allocation, rather than making
postlen==0 special). That would let the ignore-whitespace code path use
the optimization, too (when it's possible).

By the way, I notice that when update_pre_post_images does allocate, the
old value of postimage->buf is lost. It looks like that is not leaked,
because it was pointing to a strbuf (newlines in apply_one_fragment)
that we are going to release anyway afterwards. But that means nobody is
freeing postimage->buf, which means that our newly malloc'd version is
getting leaked.

-Peff