Re: [PATCH] apply: reallocate the postimage buffer when needed

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Junio C Hamano <gitster@xxxxxxxxx> writes:

> Carlos Martín Nieto <cmn@xxxxxxxx> writes:
>
>> Blame says Junio and Giuseppe were the last ones to touch this part of
>> the code, so there you go.
>
> Whatever you do in your fix, this comment block needs to be updated:
>
> 	/*
> 	 * Adjust the common context lines in postimage. This can be
> 	 * done in-place when we are just doing whitespace fixing,
> 	 * which does not make the string grow, but needs a new buffer
> 	 * when ignoring whitespace causes the update, since in this case
> 	 * we could have e.g. tabs converted to multiple spaces.
> 	 * We trust the caller to tell us if the update can be done
> 	 * in place (postlen==0) or not.
> 	 */
>
> The second sentence used to be true for a long time (if you indented
> your line with too many spaces, we removed them and replaced with
> fewer number of tabs; if you had spaces before a tab, we removed
> them; if you added unnecessary whitespaces at the end, we removed
> them), but ceased to be so when Python style "indent must be spaces"
> was added.
>
> So I think this either always needs to re-allocate, or the caller
> has to tell it by other means than "!postlen" the need for
> reallocation.
>
> I wasn't involved in the "apply while ignoring whitespace
> differences", so Giuseppe may be able to notice other mode of
> beakages in this and fuzzy_matchlines() function.  The commit to be
> stared at is 86c91f9 (git apply: option to ignore whitespace
> differences, 2009-08-04).

Looking at the call site of update_pre_post_images() again, I think
the call made from this part may also need to be updated for "indent
must be spaces" that was added by 3e3ec2a (whitespace: add
tab-in-indent error class, 2010-04-03) and 4e35c51 (whitespace: add
tab-in-indent support for --whitespace=fix, 2010-04-03):

	... code to look at preimage and img, checking if the lines
        ... match if the whitespace breakages in the preimage line
        ... were fixed is here.
	/*
	 * Yes, the preimage is based on an older version that still
	 * has whitespace breakages unfixed, and fixing them makes the
	 * hunk match.  Update the context lines in the postimage.
	 */
	fixed_buf = strbuf_detach(&fixed, &fixed_len);
	update_pre_post_images(preimage, postimage,
			       fixed_buf, fixed_len, 0);

The logic to match the preimage and img is correct and does not
overflow anything; the fixing of preimage and img is done with
ws_fix_copy() that uses strbuf.

But fixed_buf and fixed_len may be longer than the original length
of preimage buffer when "indent must be spaces" is in effect, and
that would mean context lines in the postimage may have to also
grow. The call to update_pre_post_images() must be telling how big
the postimage with fixed context lines will be, not passing 0 to say
"I know it will not grow", because that is no longer true these
days.
--
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


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]