Re: [ANNOUNCE] GIT 1.5.4-rc3

From: René Scharfe <rene.scharfe@lsrfire.ath.cx>
Date: Jan 12, 2008, 13:01 UTC
Message-ID: <4788BA15.3050804@lsrfire.ath.cx>
In-Reply-To: <20080112090432.GA6134@coredump.intra.peff.net>

Jeff King schrieb:

> Thus allocating "seen" based upon just argc is wrong, since
> if argc == 0, then we still have one pathspec, the prefix,
> but we don't allocate any space in "seen".

Yes, all the other callers count the number of elements in pathspec and allocate seen accordingly. We could do that. Or we could extend match_pathspec() to allow seen to be a NULL pointer, for those cases where this result isn't needed.

This patch is for discussion, only, because it touches central, non-buggy code and where in bugfix only mode. And I'm not really sure git-clean ignoring non-matching pathspecs (as it does now, even without this patch) is really what we want.

 builtin-clean.c |    8 +-------
 dir.c           |   26 ++++++++++++++++----------
 2 files changed, 17 insertions(+), 17 deletions(-)

diff --git a/builtin-clean.c b/builtin-clean.c
index 6cad8ea..40d976c 100644
--- a/builtin-clean.c
+++ b/builtin-clean.c
@@ -35,7 +35,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	const char *path, *base;
 	static const char **pathspec;
 	int prefix_offset = 0;
-	char *seen = NULL;
 	struct option options[] = {
 		OPT__QUIET(&quiet),
 		OPT__DRY_RUN(&show_only),
@@ -89,9 +88,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 	read_directory(&dir, path, base, baselen, pathspec);
 	strbuf_init(&directory, 0);
 
-	if (pathspec)
-		seen = xmalloc(argc);
-
 	for (i = 0; i < dir.nr; i++) {
 		struct dir_entry *ent = dir.entries[i];
 		int len, pos, matches;
@@ -125,9 +121,8 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			continue;
 
 		if (pathspec) {
-			memset(seen, 0, argc);
 			matches = match_pathspec(pathspec, ent->name, ent->len,
-						 baselen, seen);
+						 baselen, NULL);
 		} else {
 			matches = 0;
 		}
@@ -165,7 +160,6 @@ int cmd_clean(int argc, const char **argv, const char *prefix)
 			unlink(ent->name);
 		}
 	}
-	free(seen);
 
 	strbuf_release(&directory);
 	return 0;
diff --git a/dir.c b/dir.c
index 3e345c2..6af70ac 100644
--- a/dir.c
+++ b/dir.c
@@ -88,32 +88,38 @@ static int match_one(const char *match, const char *name, int namelen)
 
 /*
  * Given a name and a list of pathspecs, see if the name matches
- * any of the pathspecs.  The caller is also interested in seeing
- * all pathspec matches some names it calls this function with
- * (otherwise the user could have mistyped the unmatched pathspec),
- * and a mark is left in seen[] array for pathspec element that
- * actually matched anything.
+ * any of the pathspecs.
+ * If the caller is interested in which of the pathspecs matched,
+ * seen needs to point to a char array with as many members as there
+ * are pathspecs.  A mark is left at the nth element of the nth
+ * pathspec matched the name.  The marks are not reset, so the
+ * caller can check, after matching a list of files, if there are
+ * any unmatched pathspecs (which might have been mistyped).
  */
 int match_pathspec(const char **pathspec, const char *name, int namelen, int prefix, char *seen)
 {
-	int retval;
-	const char *match;
+	int retval = 0;
 
 	name += prefix;
 	namelen -= prefix;
 
-	for (retval = 0; (match = *pathspec++) != NULL; seen++) {
+	while (*pathspec) {
+		const char *match = *pathspec++;
 		int how;
-		if (retval && *seen == MATCHED_EXACTLY)
+
+		if (retval && seen && *seen == MATCHED_EXACTLY)
 			continue;
+
 		match += prefix;
 		how = match_one(match, name, namelen);
 		if (how) {
 			if (retval < how)
 				retval = how;
-			if (*seen < how)
+			if (seen && *seen < how)
 				*seen = how;
 		}
+		if (seen)
+			seen++;
 	}
 	return retval;
 }