All of lore.kernel.org
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: Jeff King <peff@peff.net>
Cc: Stepan Kasal <kasal@ucw.cz>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	git@vger.kernel.org,
	Jean-Jacques Lafay <jeanjacques.lafay@gmail.com>
Subject: Re: [PATCH v2] git tag --contains : avoid stack overflow
Date: Wed, 23 Apr 2014 13:48:05 -0700	[thread overview]
Message-ID: <xmqqk3afydq2.fsf@gitster.dls.corp.google.com> (raw)
In-Reply-To: <20140423191628.GA20596@sigill.intra.peff.net> (Jeff King's message of "Wed, 23 Apr 2014 15:16:28 -0400")

Jeff King <peff@peff.net> writes:

> On Wed, Apr 23, 2014 at 12:12:14PM -0700, Junio C Hamano wrote:
>
>> > +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?
>
> I don't think so. The point is that we _must_ use bash here, not any
> POSIX shell.

Sorry, but I do not understand.  Isn't what you want "any POSIX
shell with 'ulimit -s 64' supported"?

    $ dash -c 'ulimit -s && ulimit -s 64 && ulimit -s'
    8192
    64

> We want to run "some bash" if we can. We may pick a bash on the user's
> PATH that is not what they put into $SHELL_PATH, but that should be
> relatively rare. And the consequence is that either that bash works fine
> and we run the test, or it does not, and we skip the test.
>
>> 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"
>
> That's a much more direct test. I like it (aside from the $SHELL_PATH
> thing as described above).

Still puzzled.

  reply	other threads:[~2014-04-23 20:48 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-04-16 14:15 [PATCH] git tag --contains : avoid stack overflow Stepan Kasal
2014-04-16 15:46 ` Jeff King
2014-04-17 17:31   ` Johannes Schindelin
2014-04-17 21:32     ` Jeff King
2014-04-17 21:52       ` Johannes Schindelin
2014-04-17 21:58         ` Jeff King
2014-04-23  7:53           ` [PATCH v2] " Stepan Kasal
2014-04-23 14:28             ` Johannes Schindelin
2014-04-23 15:45               ` Stepan Kasal
2014-04-23 19:12             ` Junio C Hamano
2014-04-23 19:16               ` Jeff King
2014-04-23 20:48                 ` Junio C Hamano [this message]
2014-04-23 20:55                   ` Jeff King
2014-04-23 21:05                     ` Junio C Hamano
2014-04-24 12:20                       ` Stepan Kasal
2014-04-24 12:24                         ` [PATCH v3] git tag --contains: " Stepan Kasal
2014-04-25  5:54                           ` Jeff King
2014-09-20 18:18                           ` Andreas Schwab
2014-09-23 16:05                             ` Jeff King
2014-09-23 21:48                               ` Andreas Schwab
2014-09-23 22:41                                 ` Junio C Hamano
2014-04-23 19:59               ` [PATCH v2] git tag --contains : " Stepan Kasal
2014-04-23 19:17             ` Jeff King
2014-04-23 21:14               ` Johannes Schindelin

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=xmqqk3afydq2.fsf@gitster.dls.corp.google.com \
    --to=gitster@pobox.com \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=jeanjacques.lafay@gmail.com \
    --cc=kasal@ucw.cz \
    --cc=peff@peff.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.