problem with git-diff-tree -S

problem with git-diff-tree -S

[Paul Mackerras]

I'm trying to implement a find function in gitk that will find commits
where the patch includes a particular string, using git-diff-tree.
It's not doing what I expect - am I expecting the wrong thing, or is
there a bug in git-diff-tree?

As an example, take commit 92a582ed2757456ca9599f8b4ea2064f2154eb02,
"[IA64] sparse cleanup of TIOCA files".  If I put that ID in a file
called id and do:

git-diff-tree --stdin -p <id

the resulting patch includes the lines:

-uint64_t
+static uint64_t

+int sn_topology_open(struct inode *inode, struct file *file);
+int sn_topology_release(struct inode *inode, struct file *file);

+extern struct list_head tioca_list;

Now if I do:

git-diff-tree --stdin -r -s -Ssn_topology <id

I get the ID printed to stdout, but if I do:

git-diff-tree --stdin -r -s -Stioca <id

I get nothing on stdout, and similarly if I use "-Sstatic".  What's
going on?

Paul.

[PATCH] diffcore-pickaxe: switch to "counting" behaviour.

[Junio C Hamano]

Instead of finding old/new pair that one side has and the
other side does not have the specified string, find old/new pair
that contains the specified string as a substring different
number of times.  This would still not catch a case where you
introduce two static variable declarations and remove two static
function definitions from a file with -S"static", but would make
it behave a bit more intuitively.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---

*** Linus, the "counting" behaviour that came up in our private
*** exchange was brought up independently by Paul.  I think it
*** is a good compromise.

 diffcore-pickaxe.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

7532cfc15932595d679c16ae007f9a910a5b1d1e
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -5,19 +5,30 @@
 #include "diff.h"
 #include "diffcore.h"
 
-static int contains(struct diff_filespec *one,
-		    const char *needle, unsigned long len)
+static unsigned int contains(struct diff_filespec *one,
+			     const char *needle, unsigned long len)
 {
+	unsigned int cnt;
 	unsigned long offset, sz;
 	const char *data;
 	if (diff_populate_filespec(one, 0))
 		return 0;
+
 	sz = one->size;
 	data = one->data;
-	for (offset = 0; offset + len <= sz; offset++)
-		     if (!strncmp(needle, data + offset, len))
-			     return 1;
-	return 0;
+	cnt = 0;
+
+	/* Yes, I've heard of strstr(), but the thing is *data may
+	 * not be NUL terminated.  Sue me.
+	 */
+	for (offset = 0; offset + len <= sz; offset++) {
+		/* we count non-overlapping occurrences of needle */
+		if (!memcmp(needle, data + offset, len)) {
+			offset += len - 1;
+			cnt++;
+		}
+	}
+	return cnt;
 }
 
 void diffcore_pickaxe(const char *needle, int opts)

[PATCH] diffcore-pickaxe: switch to "counting" behaviour.

[Junio C Hamano]

Instead of finding old/new pair that one side has and the
other side does not have the specified string, find old/new pair
that contains the specified string as a substring different
number of times.  This would still not catch a case where you
introduce two static variable declarations and remove two static
function definitions from a file with -S"static", but would make
it behave a bit more intuitively.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
*** This was brought up as a possible improvement for the first
*** question when you asked me two questions a couple of weeks
*** ago.  Interestingly enough, Paul Mackerras independently
*** suggested the same approach to make pickaxe more intuitive
*** for gitk users.

 diffcore-pickaxe.c |   23 +++++++++++++++++------
 1 files changed, 17 insertions(+), 6 deletions(-)

d245f19da1e36a70e904f2414d815fc06bfe09d8
diff --git a/diffcore-pickaxe.c b/diffcore-pickaxe.c
--- a/diffcore-pickaxe.c
+++ b/diffcore-pickaxe.c
@@ -5,19 +5,30 @@
 #include "diff.h"
 #include "diffcore.h"
 
-static int contains(struct diff_filespec *one,
-		    const char *needle, unsigned long len)
+static unsigned int contains(struct diff_filespec *one,
+			     const char *needle, unsigned long len)
 {
+	unsigned int cnt;
 	unsigned long offset, sz;
 	const char *data;
 	if (diff_populate_filespec(one, 0))
 		return 0;
+
 	sz = one->size;
 	data = one->data;
-	for (offset = 0; offset + len <= sz; offset++)
-		     if (!strncmp(needle, data + offset, len))
-			     return 1;
-	return 0;
+	cnt = 0;
+
+	/* Yes, I've heard of strstr(), but the thing is *data may
+	 * not be NUL terminated.  Sue me.
+	 */
+	for (offset = 0; offset + len <= sz; offset++) {
+		/* we count non-overlapping occurrences of needle */
+		if (!memcmp(needle, data + offset, len)) {
+			offset += len - 1;
+			cnt++;
+		}
+	}
+	return cnt;
 }
 
 void diffcore_pickaxe(const char *needle, int opts)