Re: [PATCH v4 05/16] ref-filter.c: parameterize match functions over patterns

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Tue, Jun 20, 2023 at 10:21:21AM -0400, Taylor Blau wrote:

> Once we start passing either in, `match_pattern()` will have little to
> do with a particular `struct ref_filter *` instance. To clarify this,
> drop it from the argument list, and replace it with the only bit of the
> `ref_filter` that we care about (`filter->ignore_case`).

Makes sense, but...

> @@ -2134,9 +2134,10 @@ static int match_pattern(const struct ref_filter *filter, const char *refname)
>   * matches a pattern "refs/heads/" but not "refs/heads/m") or a
>   * wildcard (e.g. the same ref matches "refs/heads/m*", too).
>   */
> -static int match_name_as_path(const struct ref_filter *filter, const char *refname)
> +static int match_name_as_path(const struct ref_filter *filter,
> +			      const char **pattern,
> +			      const char *refname)

...wouldn't we then want to do the same for match_name_as_path()?

I.e., this:

diff --git a/ref-filter.c b/ref-filter.c
index 6aacb99be7..cf10c753e2 100644
--- a/ref-filter.c
+++ b/ref-filter.c
@@ -2134,14 +2134,13 @@ static int match_pattern(const char **patterns, const char *refname,
  * matches a pattern "refs/heads/" but not "refs/heads/m") or a
  * wildcard (e.g. the same ref matches "refs/heads/m*", too).
  */
-static int match_name_as_path(const struct ref_filter *filter,
-			      const char **pattern,
-			      const char *refname)
+static int match_name_as_path(const char **pattern, const char *refname,
+			      int ignore_case)
 {
 	int namelen = strlen(refname);
 	unsigned flags = WM_PATHNAME;
 
-	if (filter->ignore_case)
+	if (ignore_case)
 		flags |= WM_CASEFOLD;
 
 	for (; *pattern; pattern++) {
@@ -2166,7 +2165,8 @@ static int filter_pattern_match(struct ref_filter *filter, const char *refname)
 	if (!*filter->name_patterns)
 		return 1; /* No pattern always matches */
 	if (filter->match_as_path)
-		return match_name_as_path(filter, filter->name_patterns, refname);
+		return match_name_as_path(filter->name_patterns, refname,
+					  filter->ignore_case);
 	return match_pattern(filter->name_patterns, refname,
 			     filter->ignore_case);
 }

Also, I noticed that you declared it as "const int ignore_case" in
match_pattern(). That's not wrong, but we usually do not bother (it is
passed by value, so const-ness is irrelevant to the caller, and the
compiler can see inside the function that the value is not changed and
optimize appropriately).

-Peff



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux