Re: [PATCH v2] git tag --contains : avoid stack overflow

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

 



Stepan Kasal <kasal@xxxxxx> writes:

[Administrivia: please refrain from using Mail-Followup-To to
deflect an attempt to directly respond to you; it will waste time of
other people while it may be saving your time].

> From: Jean-Jacques Lafay <jeanjacques.lafay@xxxxxxxxx>
>
> In large repos, the recursion implementation of contains(commit,
> commit_list) may result in a stack overflow. Replace the recursion with
> a loop to fix it.
>
> This problem is more apparent on Windows than on Linux, where the stack
> is more limited by default.
>
> See also this thread on the msysGit list:
>
> 	https://groups.google.com/d/topic/msysgit/FqT6boJrb2g/discussion
>
> [jes: re-written to imitate the original recursion more closely]
>
> Thomas Braun pointed out several documentation shortcomings.
>
> Tests are run only if ulimit -s is available.  This means they cannot
> be run on Windows.
>
> Signed-off-by: Jean-Jacques Lafay <jeanjacques.lafay@xxxxxxxxx>
> Signed-off-by: Johannes Schindelin <johannes.schindelin@xxxxxx>
> Tested-by: Stepan Kasal <kasal@xxxxxx>

Thanks.

> diff --git a/t/t7004-tag.sh b/t/t7004-tag.sh
> index 143a8ea..db82f6d 100755
> --- a/t/t7004-tag.sh
> +++ b/t/t7004-tag.sh
> @@ -1423,4 +1423,27 @@ EOF
>  	test_cmp expect actual
>  '
>  
> +ulimit_stack="ulimit -s 64"
> +test_lazy_prereq ULIMIT 'bash -c "'"$ulimit_stack"'"'

With this implementaion, ULIMIT implies bash, and we use bash that
appears on user's PATH that may not be the one the user chose to run
git with.  Can't we fix both of them by using $SHELL_PATH?

> +>expect

Move this inside test_expect_success?

> +# we require bash and ulimit, this excludes Windows
> +test_expect_success ULIMIT '--contains works in a deep repo' '
> +	i=1 &&
> +	while test $i -lt 4000
> +	do
> +		echo "commit refs/heads/master
> +committer A U Thor <author@xxxxxxxxxxx> $((1000000000 + $i * 100)) +0200
> +data <<EOF
> +commit #$i
> +EOF"
> +		test $i = 1 && echo "from refs/heads/master^0"
> +		i=$(($i + 1))
> +	done | git fast-import &&
> +	git checkout master &&
> +	git tag far-far-away HEAD^ &&
> +	bash -c "'"$ulimit_stack"' && git tag --contains HEAD >actual" &&

So this runs a separate "bash", the only thing which does is to run
a small script that gives a small stack to itself and exit, and then
run "git tag" in the original shell?

Ahh, no, I am mis-pairing the quotes.

How about doing it along this line instead?

	run_with_limited_stack () {
		"$SHELL_PATH" -c "ulimit -s 64 && $*"
	}

	test_lazy_prereq ULIMIT "run_with_limited_stack true"

	test_expect_success ULIMIT '...' '
        	>expect &&
		i=1 &&
                ...
                done | git-fast-import &&
                git tag far-far-away HEAD^ &&
                run_with_limited_stack "git tag --contains HEAD" >actual &&
		test_cmp expect actual
	'

--
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]