On Mon, Sep 20, 2021 at 05:57:36PM +0000, Derrick Stolee via GitGitGadget wrote: > From: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > > Add a test to t1091-sparse-checkout-builtin.sh that would result in an > infinite loop and out-of-memory error before this change. The issue > relies on having non-cone-mode patterns while trying to modify the > patterns in cone-mode. > > The fix is simple, allowing us to break from the loop when the input > path does not contain a slash, as the "dir" pattern we added does not. > > This is only a fix to the critical out-of-memory error. A better > response to such a strange state will follow in a later change. > > Reported-by: Calbabreaker <calbabreaker@xxxxxxxxx> > Helped-by: Taylor Blau <me@xxxxxxxxxxxx> > Signed-off-by: Derrick Stolee <dstolee@xxxxxxxxxxxxx> > --- > builtin/sparse-checkout.c | 2 +- > t/t1091-sparse-checkout-builtin.sh | 8 ++++++++ > 2 files changed, 9 insertions(+), 1 deletion(-) > > diff --git a/builtin/sparse-checkout.c b/builtin/sparse-checkout.c > index 8ba9f13787b..b45fd97a98b 100644 > --- a/builtin/sparse-checkout.c > +++ b/builtin/sparse-checkout.c > @@ -389,7 +389,7 @@ static void insert_recursive_pattern(struct pattern_list *pl, struct strbuf *pat > char *oldpattern = e->pattern; > size_t newlen; > > - if (slash == e->pattern) > + if (!slash || slash == e->pattern) > break; If we are preparing to make it so that we do not blindly copy patterns from a sparse checkout without cone mode enabled, then wouldn't this new case be a BUG()? Of course, users could still tweak the contents of their sparse-checkout file as they wish, but I'd expect that we'd catch those cases, too (i.e., by validating the contents of the existing sparse-checkout before calling this function). > diff --git a/t/t1091-sparse-checkout-builtin.sh b/t/t1091-sparse-checkout-builtin.sh > index 38fc8340f5c..a429d2cc671 100755 > --- a/t/t1091-sparse-checkout-builtin.sh > +++ b/t/t1091-sparse-checkout-builtin.sh > @@ -103,6 +103,14 @@ test_expect_success 'clone --sparse' ' > check_files clone a > ' > > +test_expect_success 'switching to cone mode with non-cone mode patterns' ' > + git init bad-patterns && > + git -C bad-patterns sparse-checkout init && > + git -C bad-patterns sparse-checkout add dir && > + git -C bad-patterns config core.sparseCheckoutCone true && Makes sense that we'd want to update the config rather than call "init --cone" here, since a subsequent patch would change the thing that this is testing (from "doesn't OOM in the above-described situation" to "clears the existing contents of your sparse-checkout"). > + git -C bad-patterns sparse-checkout add dir > +' > + In other series I've suggested a cosmetic change to move all of these to a sub-shell that begins with "cd bad-patterns &&", but obviously that is a relatively unimportant suggestion. Thanks, Taylor
