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

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

 



On 10 November 2012 21:13, Jean-Jacques Lafay
<jeanjacques.lafay@xxxxxxxxx> wrote:
> Le samedi 10 novembre 2012 21:00:10 UTC+1, Philip Oakley a écrit :
>>
>> From: "Jean-Jacques Lafay" <jeanjacq...@xxxxxxxxx> Sent: Saturday,
>> November 10, 2012 5:36 PM
>> > 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.
>> >
>> > Signed-off-by: Jean-Jacques Lafay <jeanjacq...@xxxxxxxxx>
>>
>> This is a change to the main git code so it is better to submit it to
>> the main git list at g...@xxxxxxxxxxxxxxx (plain text, no HTML, first
>> post usually has a delay ;-)
>>
>> It sounds reasonable but others may have opinions and comments.
>>
>> Have you actually suffered from the stack overflow issue? You only
>> suggest it as a possibility, rather than a real problem.
>>
>> Philip
>
>
> Yes, it actually crashed on me, and since the call is made by GitExtension
> while browsing the commit history, it was quickly annoying to have windows
> popping a "git.exe stopped working" message box at my face every time I
> clicked on a line of the history ;-)  (well, you can sort of work around it
> by having the "file tree" or "diff" tab active)
>
> Coincidentally, as I was having a glance at the recent topics, I saw that
> someone else experienced it.
>
> However, I couldn't reproduce it on Linux : where the windows
> implementations crashes at a ~32000 depth (*not* exactly 32768, mind you),
> on linux it happily went through 100000 commits. I didn't take time to look
> much further, but maybe on my 64 bit Linux VM, the process can afford to
> reserve a much bigger address range for the stack of each thread than the
> 1Mb given to 32 bit processes on windows.
>
> Jean-Jacques.

I checked this on msysGit - the test causes a crash on Windows when
using the 1.8.0 release and recompiling with this patch applied fixes
this. As mentioned, its best to have this reviewed upstream too as its
likely that windows just has a smaller stack so causes the crash
earlier.
--
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]