threads / patch / 6537

patchdiffcore-pickaxe: fix infinite loop on zero-length needle

Subject: [PATCH] diffcore-pickaxe: fix infinite loop on zero-length needle

## tl;dr

3 messages between Jan 26, 2007 and Jan 26, 2007. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Jeff King· Jan 26, 2007, 04:48 UTC · lore
The "contains" algorithm runs into an infinite loop if the needle string
has zero length. The loop could be modified to handle this, but it makes
more sense to simply have an empty needle return no matches. Thus, a
command like
  git log -S
produces no output.
We place the check at the top of the function so that we get the same
results with or without --pickaxe-regex. Note that until now,
  git log -S --pickaxe-regex
would match everything, not nothing.

Arguably, an empty pickaxe string should simply produce an error message; however, this is still a useful assertion to add to the algorithm at this layer of the code.

Noticed by Bill Lear.
Signed-off-by: Jeff King <peff@peff.net>
---
On Thu, Jan 25, 2007 at 06:06:51PM -0600, Bill Lear wrote:
> git log -S --check foo
>
> and the thing went off into outer space.  Now at over 2 1/2 minutes of
> CPU time on my 2 Ghz Opteron box...
 diffcore-pickaxe.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
Show changes to diffcore-pickaxe.c +2 −0
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
index de44ada..286919e 100644
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -14,6 +14,8 @@ static unsigned int contains(struct diff_filespec *one,
 	const char *data;
 	if (diff_populate_filespec(one, 0))
 		return 0;
+	if (!len)
+		return 0;
 
 	sz = one->size;
 	data = one->data;

-- 
1.5.0.rc2.gccd57
Jeff King· Jan 26, 2007, 04:56 UTC · re: Jeff King · lore

Re: [PATCH] diffcore-pickaxe: fix infinite loop on zero-length needle

On Thu, Jan 25, 2007 at 11:48:58PM -0500, Jeff King wrote:
Show 10 quoted lines
> has zero length. The loop could be modified to handle this, but it makes
> more sense to simply have an empty needle return no matches. Thus, a
> command like
>   git log -S
> produces no output.
> 
> We place the check at the top of the function so that we get the same
> results with or without --pickaxe-regex. Note that until now,
>   git log -S --pickaxe-regex
> would match everything, not nothing.

My assumption is that nobody is really depending on this behavior (since, after all, without --pickaxe-regex it caused an infinite loop!). If you want to maintain consistency, the conditional should simply return 1 instead of 0 (no callers of contains() actually care about the count value).

-Peff

← back to recent threads