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 -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html