Skip to contentSkip to conversation

Create ref-filter from for-each-ref

PATCH Superseded Karthik Nayak 4 participants 32 messages Jun 15, 2016 Kbuild

v7 of 8 Superseded Latest is v8 →

lore
Share & tools
lore
#1 lore raw

PATCH v7 1/12 for-each-ref: extract helper functions out of grab_single_ref()

Extract two helper functions out of grab_single_ref(). Firstly, new_refinfo() which is used to allocate memory for a new refinfo structure and copy the objectname, refname and flag to it. Secondly, match_name_as_path() which when given an array of patterns and the refname checks if the refname matches any of the patterns given while the pattern is a pathname, also supports wildcard characters.

This is a preperatory patch for restructuring 'for-each-ref' and eventually moving most of it to 'ref-filter' to provide the functionality to similar commands via public API's.

---
 builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 23 deletions(-)
builtin/for-each-ref.c +41 -23
index f7e51a7..67c8b62 100644
@@ -851,6 +851,44 @@ struct grab_ref_cbdata {
 };
 
 /*
+ * Return 1 if the refname matches with one of the patterns,
+ * otherwise 0.  The patterns can be literal prefix (e.g. a
+ * refname "refs/heads/master" matches a pattern "refs/heads/")
+ * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
+ */
+static int match_name_as_path(const char **pattern, const char *refname)
+{
+	int namelen = strlen(refname);
+	for (; *pattern; pattern++) {
+		const char *p = *pattern;
+		int plen = strlen(p);
+
+		if ((plen <= namelen) &&
+		    !strncmp(refname, p, plen) &&
+		    (refname[plen] == '\0' ||
+		     refname[plen] == '/' ||
+		     p[plen-1] == '/'))
+			return 1;
+		if (!wildmatch(p, refname, WM_PATHNAME, NULL))
+			return 1;
+	}
+	return 0;
+}
+
+/* Allocate space for a new refinfo and copy the objectname and flag to it */
+static struct refinfo *new_refinfo(const char *refname,
+				   const unsigned char *objectname,
+				   int flag)
+{
+	struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+	ref->refname = xstrdup(refname);
+	hashcpy(ref->objectname, objectname);
+	ref->flag = flag;
+
+	return ref;
+}
+
+/*
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
@@ -866,35 +904,15 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 		  return 0;
 	}
 
-	if (*cb->grab_pattern) {
-		const char **pattern;
-		int namelen = strlen(refname);
-		for (pattern = cb->grab_pattern; *pattern; pattern++) {
-			const char *p = *pattern;
-			int plen = strlen(p);
-
-			if ((plen <= namelen) &&
-			    !strncmp(refname, p, plen) &&
-			    (refname[plen] == '\0' ||
-			     refname[plen] == '/' ||
-			     p[plen-1] == '/'))
-				break;
-			if (!wildmatch(p, refname, WM_PATHNAME, NULL))
-				break;
-		}
-		if (!*pattern)
-			return 0;
-	}
+	if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
+		return 0;
 
 	/*
 	 * We do not open the object yet; sort may only need refname
 	 * to do its job and the resulting list may yet to be pruned
 	 * by maxcount logic.
 	 */
-	ref = xcalloc(1, sizeof(*ref));
-	ref->refname = xstrdup(refname);
-	hashcpy(ref->objectname, oid->hash);
-	ref->flag = flag;
+	ref = new_refinfo(refname, oid->hash, flag);
 
 	cnt = cb->grab_cnt;
 	REALLOC_ARRAY(cb->grab_array, cnt + 1);
Signature · 1 line
2.4.2
#2 lore raw

PATCH v7 2/12 for-each-ref: clean up code

In 'grab_single_ref()' remove the extra count variable 'cnt' and use the variable 'grab_cnt' of structure 'grab_ref_cbdata' directly instead.

---
 builtin/for-each-ref.c | 7 ++-----
 1 file changed, 2 insertions(+), 5 deletions(-)
builtin/for-each-ref.c +2 -5
index 67c8b62..0dd2df2 100644
@@ -897,7 +897,6 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 {
 	struct grab_ref_cbdata *cb = cb_data;
 	struct refinfo *ref;
-	int cnt;
 
 	if (flag & REF_BAD_NAME) {
 		  warning("ignoring ref with broken name %s", refname);
@@ -914,10 +913,8 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 */
 	ref = new_refinfo(refname, oid->hash, flag);
 
-	cnt = cb->grab_cnt;
-	REALLOC_ARRAY(cb->grab_array, cnt + 1);
-	cb->grab_array[cnt++] = ref;
-	cb->grab_cnt = cnt;
+	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
+	cb->grab_array[cb->grab_cnt++] = ref;
 	return 0;
 }
 
Signature · 1 line
2.4.2
#3 lore raw

PATCH v7 3/12 for-each-ref: change comment in ref_sort

In reply to Karthik Nayak

The comment in 'ref_sort' hasn't been changed 9f613dd. Change the comment to reflect changes made in the code since 9f613dd.

---
 builtin/for-each-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
builtin/for-each-ref.c +1 -1
index 0dd2df2..bfad03f 100644
@@ -27,7 +27,7 @@ struct atom_value {
 
 struct ref_sort {
 	struct ref_sort *next;
-	int atom; /* index into used_atom array */
+	int atom; /* index into 'struct atom_value *' array */
 	unsigned reverse : 1;
 };
 
Signature · 1 line
2.4.2
#4 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 1 line · Karthik Nayak

The comment in 'ref_sort' hasn't been changed 9f613dd.

Bad grammar? "hasn't been changed since 9f613dd", perhaps?

But more importantly, don't just give an abbreviated object name. I think "the comment hasn't changed since the for-each-ref command was originally introduced" is what you meant to say, and it is OK to append "since 9f613ddd (Add git-for-each-ref: helper for language bindings, 2006-09-15)" to that sentence as a supporting material.

Quoted 2 lines

Change the comment to reflect changes made in the code since 9f613dd.

What change since 9f613dd do you have in mind, exactly, though?

I do not think the fact that this field indexes into used_atom[] array has ever changed during the life of this implementation. I see "static const char **used_atom;" in builtin/for-each-ref.c still in the 'master', and that is the array that holds the atoms that are used by the end-user request.

So I do not think "The comment was there from the beginning, it described the initial implementation, the implementation was updated and the comment has become stale" is a good justification for this change, as I do not think that is what has happened here.

You may be changing used_atom to something else later in your series, but then isn't that commit the appropriate place to update this comment?

Quoted 19 lines
---
 builtin/for-each-ref.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
builtin/for-each-ref.c +1 -1
index 0dd2df2..bfad03f 100644
@@ -27,7 +27,7 @@ struct atom_value {
 
 struct ref_sort {
 	struct ref_sort *next;
-	int atom; /* index into used_atom array */
+	int atom; /* index into 'struct atom_value *' array */
 	unsigned reverse : 1;
 };
#5 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 5 lines · Junio C Hamano
Quoted 1 line · Karthik Nayak

The comment in 'ref_sort' hasn't been changed 9f613dd.

Bad grammar? "hasn't been changed since 9f613dd", perhaps?

Yes! thanks :)

Quoted 7 lines

But more importantly, don't just give an abbreviated object name. I think "the comment hasn't changed since the for-each-ref command was originally introduced" is what you meant to say, and it is OK to append "since 9f613ddd (Add git-for-each-ref: helper for language bindings, 2006-09-15)" to that sentence as a supporting material.

Ok will do.

Quoted 4 lines
Quoted 2 lines

Change the comment to reflect changes made in the code since 9f613dd.

What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which later was removed. Hence the comment becomes obsolete.

Quoted 7 lines

I do not think the fact that this field indexes into used_atom[] array has ever changed during the life of this implementation. I see "static const char **used_atom;" in builtin/for-each-ref.c still in the 'master', and that is the array that holds the atoms that are used by the end-user request.

Quoted 9 lines

So I do not think "The comment was there from the beginning, it described the initial implementation, the implementation was updated and the comment has become stale" is a good justification for this change, as I do not think that is what has happened here.

You may be changing used_atom to something else later in your series, but then isn't that commit the appropriate place to update this comment?

But isn't that what happened here, the code was altered but the comment was left the way it is.

What do you suggest I do?

Signature · 2 lines
Regards,
Karthik
#6 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 4 lines · Karthik Nayak
Quoted 1 line

What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which later was removed. Hence the comment becomes obsolete.

Later in which commit? In builtin/for-each-ref.c in the version after applying patches 1-3 of this series on top of master, I still see used_atom[] array there, so...?

#7 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 11 lines · Junio C Hamano
Quoted 4 lines · Karthik Nayak
Quoted 1 line

What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which later was removed. Hence the comment becomes obsolete.

Later in which commit? In builtin/for-each-ref.c in the version after applying patches 1-3 of this series on top of master, I still see used_atom[] array there, so...?

Uh! My bad.
Ignore this! I think I got confused, On having a look now that patch is not needed. Sorry.

Signature · 2 lines
Regards,
Karthik
#8 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 11 lines · Karthik Nayak
Quoted 10 lines · Junio C Hamano
Quoted 5 lines · Karthik Nayak
Quoted 1 line

What change since 9f613dd do you have in mind, exactly, though?

Well initially the atoms were indexed into used_atom array, which later was removed. Hence the comment becomes obsolete.

Quoted 8 lines
Quoted 4 lines

Later in which commit? In builtin/for-each-ref.c in the version after applying patches 1-3 of this series on top of master, I still see used_atom[] array there, so...?

Uh! My bad.
Ignore this! I think I got confused, On having a look now that patch is not needed. Sorry.

I think it is needed later when "struct ref_sort" is moved into ref-filter.h, because then the used_atom[] array is not moved.

So either:

1) you update the comment when you move "struct ref_sort" into ref-filter.h, but then the downside is that there is not only code movement in the patch that moves "struct ref_sort" into ref-filter.h, or

2) you explain that, as "struct ref_sort" will be moved later while the used_atom[] array will not be moved, the direct connection between the comment and used_atom[] will be broken, and you need to prepare for that because you want to avoid solution 1) as you want only code movement when moving "struct ref_sort" into ref-filter.h.

Hope this helps,
Christian.

#9 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 2 lines · Christian Couder

I think it is needed later when "struct ref_sort" is moved into ref-filter.h, because then the used_atom[] array is not moved.

Now I am confused. used_atom[] is the mechanism we use to give a small integer to each atom used in the %(placeholder) string, so that we do not have to refer to them as "placeholder" string and we do not have to call !strcmp() to compare for equality. How can a field in ref_sort refer to it internally if used_atom[] is not visible?

Indeed, ref-filter.c after the series does have used_atom[] and get_ref_atom_value() does use atom to index into it. So these two lines do not make much sense to me. I am puzzled.

If by "moved" you are referring to the fact that the structure and its fields are exposed to the callers of the API while the implementation detail of the mechanism to give short integer to each used atom is not exposed to them, then I do agree that the comment to the structure field as the external API definition should not talk about used_atom[] array.

Perhaps in 03/12, you can stop talking about the implementation (i.e. the value is used to index into used_atom[] to get to the original string) and instead start talking about what the value means to the callers (that are still internal to for-each-ref implementation), to make things better.

Having said that, I am not sure if there is a sensible description for that field if you avoid exposing the implementation detail.

You would probably want to start by asking what that value means. For (evantual) external callers, the number is what they get from parse_ref_filter_atom(); calling that function is the only way they can get an appropriate value to stuff in the field, and parse_opt_ref_sorting() is the most likely function external callers use to make that happen.

"The number internally used to represent an attribute of a ref used when sorting the set of refs" would be one way to say what it is without exposing the implementation detail to the readers.

But does that help anybody? I doubt it. It is mouthful for external users, and it is not concrete enough for implementors.

So either

- treat ref-filter.h as information for API users, and not talk about what it means at all, or

- treat ref-filter.h as information for both API users and API implementors, and describe it as an index into used_atom[], i.e. do not change anything.

I'd say the latter is a more sensible way to go. I think it is also OK to change this comment to "index into used_atom array (internal)" when ref-filter.h is created as an external API definition.

#10 lore raw

Re: [PATCH v7 03/12] for-each-ref: change comment in ref_sort

Quoted 51 lines · Junio C Hamano
Quoted 2 lines · Christian Couder

I think it is needed later when "struct ref_sort" is moved into ref-filter.h, because then the used_atom[] array is not moved.

Now I am confused. used_atom[] is the mechanism we use to give a small integer to each atom used in the %(placeholder) string, so that we do not have to refer to them as "placeholder" string and we do not have to call !strcmp() to compare for equality. How can a field in ref_sort refer to it internally if used_atom[] is not visible?

Indeed, ref-filter.c after the series does have used_atom[] and get_ref_atom_value() does use atom to index into it. So these two lines do not make much sense to me. I am puzzled.

If by "moved" you are referring to the fact that the structure and its fields are exposed to the callers of the API while the implementation detail of the mechanism to give short integer to each used atom is not exposed to them, then I do agree that the comment to the structure field as the external API definition should not talk about used_atom[] array.

Perhaps in 03/12, you can stop talking about the implementation (i.e. the value is used to index into used_atom[] to get to the original string) and instead start talking about what the value means to the callers (that are still internal to for-each-ref implementation), to make things better.

Having said that, I am not sure if there is a sensible description for that field if you avoid exposing the implementation detail.

You would probably want to start by asking what that value means. For (evantual) external callers, the number is what they get from parse_ref_filter_atom(); calling that function is the only way they can get an appropriate value to stuff in the field, and parse_opt_ref_sorting() is the most likely function external callers use to make that happen.

"The number internally used to represent an attribute of a ref used when sorting the set of refs" would be one way to say what it is without exposing the implementation detail to the readers.

But does that help anybody? I doubt it. It is mouthful for external users, and it is not concrete enough for implementors.

So either

- treat ref-filter.h as information for API users, and not talk about what it means at all, or

Quoted 7 lines

- treat ref-filter.h as information for both API users and API implementors, and describe it as an index into used_atom[], i.e. do not change anything.

I'd say the latter is a more sensible way to go. I think it is also OK to change this comment to "index into used_atom array (internal)" when ref-filter.h is created as an external API definition.

Like you said, the comment is still relevent to the code.
So I guess of the two options suggested by you the option of keeping the comment and just adding "(internal)" while the code is moved to ref-filter.h seems to be the best solution. This would eliminate the need for PATCH 03.

--
Regards,
Karthik

#11 lore raw

PATCH v7 4/12 for-each-ref: rename 'refinfo' to 'ref_array_item'

In reply to Karthik Nayak

Rename 'refinfo' to 'ref_array_item' as a preparatory step for introduction of new structures in the forthcoming patch.

Re-order the fields in 'ref_array_item' so that refname can be eventually converted to a FLEX_ARRAY.

---
 builtin/for-each-ref.c | 40 ++++++++++++++++++++--------------------
 1 file changed, 20 insertions(+), 20 deletions(-)
builtin/for-each-ref.c +20 -20
index bfad03f..8c44b49 100644
@@ -31,12 +31,12 @@ struct ref_sort {
 	unsigned reverse : 1;
 };
 
-struct refinfo {
-	char *refname;
+struct ref_array_item {
 	unsigned char objectname[20];
 	int flag;
 	const char *symref;
 	struct atom_value *value;
+	char *refname;
 };
 
 static struct {
@@ -86,7 +86,7 @@ static struct {
  * a "*" to denote deref_tag().
  *
  * We parse given format string and sort specifiers, and make a list
- * of properties that we need to extract out of objects.  refinfo
+ * of properties that we need to extract out of objects.  ref_array_item
  * structure will hold an array of values extracted that can be
  * indexed with the "atom number", which is an index into this
  * array.
@@ -623,7 +623,7 @@ static inline char *copy_advance(char *dst, const char *src)
 /*
  * Parse the object referred by ref, and grab needed value.
  */
-static void populate_value(struct refinfo *ref)
+static void populate_value(struct ref_array_item *ref)
 {
 	void *buf;
 	struct object *obj;
@@ -835,7 +835,7 @@ static void populate_value(struct refinfo *ref)
  * Given a ref, return the value for the atom.  This lazily gets value
  * out of the object by calling populate value.
  */
-static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
+static void get_value(struct ref_array_item *ref, int atom, struct atom_value **v)
 {
 	if (!ref->value) {
 		populate_value(ref);
@@ -845,7 +845,7 @@ static void get_value(struct refinfo *ref, int atom, struct atom_value **v)
 }
 
 struct grab_ref_cbdata {
-	struct refinfo **grab_array;
+	struct ref_array_item **grab_array;
 	const char **grab_pattern;
 	int grab_cnt;
 };
@@ -875,12 +875,12 @@ static int match_name_as_path(const char **pattern, const char *refname)
 	return 0;
 }
 
-/* Allocate space for a new refinfo and copy the objectname and flag to it */
-static struct refinfo *new_refinfo(const char *refname,
-				   const unsigned char *objectname,
-				   int flag)
+/* Allocate space for a new ref_array_item and copy the objectname and flag to it */
+static struct ref_array_item *new_ref_array_item(const char *refname,
+						 const unsigned char *objectname,
+						 int flag)
 {
-	struct refinfo *ref = xcalloc(1, sizeof(struct refinfo));
+	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
 	ref->refname = xstrdup(refname);
 	hashcpy(ref->objectname, objectname);
 	ref->flag = flag;
@@ -896,7 +896,7 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 			   int flag, void *cb_data)
 {
 	struct grab_ref_cbdata *cb = cb_data;
-	struct refinfo *ref;
+	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
 		  warning("ignoring ref with broken name %s", refname);
@@ -911,14 +911,14 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 * to do its job and the resulting list may yet to be pruned
 	 * by maxcount logic.
 	 */
-	ref = new_refinfo(refname, oid->hash, flag);
+	ref = new_ref_array_item(refname, oid->hash, flag);
 
 	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
 	cb->grab_array[cb->grab_cnt++] = ref;
 	return 0;
 }
 
-static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b)
+static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
 	int cmp;
@@ -945,8 +945,8 @@ static int cmp_ref_sort(struct ref_sort *s, struct refinfo *a, struct refinfo *b
 static struct ref_sort *ref_sort;
 static int compare_refs(const void *a_, const void *b_)
 {
-	struct refinfo *a = *((struct refinfo **)a_);
-	struct refinfo *b = *((struct refinfo **)b_);
+	struct ref_array_item *a = *((struct ref_array_item **)a_);
+	struct ref_array_item *b = *((struct ref_array_item **)b_);
 	struct ref_sort *s;
 
 	for (s = ref_sort; s; s = s->next) {
@@ -957,10 +957,10 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct refinfo **refs, int num_refs)
+static void sort_refs(struct ref_sort *sort, struct ref_array_item **refs, int num_refs)
 {
 	ref_sort = sort;
-	qsort(refs, num_refs, sizeof(struct refinfo *), compare_refs);
+	qsort(refs, num_refs, sizeof(struct ref_array_item *), compare_refs);
 }
 
 static void print_value(struct atom_value *v, int quote_style)
@@ -1027,7 +1027,7 @@ static void emit(const char *cp, const char *ep)
 	}
 }
 
-static void show_ref(struct refinfo *info, const char *format, int quote_style)
+static void show_ref(struct ref_array_item *info, const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
 
@@ -1100,7 +1100,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
-	struct refinfo **refs;
+	struct ref_array_item **refs;
 	struct grab_ref_cbdata cbdata;
 
 	struct option opts[] = {
Signature · 1 line
2.4.2
#12 lore raw

PATCH v7 5/12 for-each-ref: introduce new structures for better organisation

In reply to Karthik Nayak

Introduce 'ref_filter_cbdata' which will hold 'ref_filter' (conditions to filter the refs on) and 'ref_array' (the array of ref_array_items). Modify the code to use these new structures.

This is a preparatory patch to eventually move code from 'for-each-ref' to 'ref-filter' and make it publicly available.

---
 builtin/for-each-ref.c | 54 ++++++++++++++++++++++++++++----------------------
 1 file changed, 30 insertions(+), 24 deletions(-)
builtin/for-each-ref.c +30 -24
index 8c44b49..913d757 100644
@@ -39,6 +39,20 @@ struct ref_array_item {
 	char *refname;
 };
 
+struct ref_array {
+	int nr, alloc;
+	struct ref_array_item **items;
+};
+
+struct ref_filter {
+	const char **name_patterns;
+};
+
+struct ref_filter_cbdata {
+	struct ref_array array;
+	struct ref_filter filter;
+};
+
 static struct {
 	const char *name;
 	cmp_type cmp_type;
@@ -844,12 +858,6 @@ static void get_value(struct ref_array_item *ref, int atom, struct atom_value **
 	*v = &ref->value[atom];
 }
 
-struct grab_ref_cbdata {
-	struct ref_array_item **grab_array;
-	const char **grab_pattern;
-	int grab_cnt;
-};
-
 /*
  * Return 1 if the refname matches with one of the patterns,
  * otherwise 0.  The patterns can be literal prefix (e.g. a
@@ -895,7 +903,8 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 static int grab_single_ref(const char *refname, const struct object_id *oid,
 			   int flag, void *cb_data)
 {
-	struct grab_ref_cbdata *cb = cb_data;
+	struct ref_filter_cbdata *ref_cbdata = cb_data;
+	struct ref_filter *filter = &ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -903,7 +912,7 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 		  return 0;
 	}
 
-	if (*cb->grab_pattern && !match_name_as_path(cb->grab_pattern, refname))
+	if (*filter->name_patterns && !match_name_as_path(filter->name_patterns, refname))
 		return 0;
 
 	/*
@@ -913,8 +922,8 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(cb->grab_array, cb->grab_cnt + 1);
-	cb->grab_array[cb->grab_cnt++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
+	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
 	return 0;
 }
 
@@ -957,10 +966,10 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct ref_array_item **refs, int num_refs)
+static void sort_refs(struct ref_sort *sort, struct ref_array *array)
 {
 	ref_sort = sort;
-	qsort(refs, num_refs, sizeof(struct ref_array_item *), compare_refs);
+	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
 static void print_value(struct atom_value *v, int quote_style)
@@ -1096,12 +1105,11 @@ static char const * const for_each_ref_usage[] = {
 
 int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
-	int i, num_refs;
+	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sort *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
-	struct ref_array_item **refs;
-	struct grab_ref_cbdata cbdata;
+	struct ref_filter_cbdata ref_cbdata;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -1139,17 +1147,15 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&cbdata, 0, sizeof(cbdata));
-	cbdata.grab_pattern = argv;
-	for_each_rawref(grab_single_ref, &cbdata);
-	refs = cbdata.grab_array;
-	num_refs = cbdata.grab_cnt;
+	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
+	ref_cbdata.filter.name_patterns = argv;
+	for_each_rawref(grab_single_ref, &ref_cbdata);
 
-	sort_refs(sort, refs, num_refs);
+	sort_refs(sort, &ref_cbdata.array);
 
-	if (!maxcount || num_refs < maxcount)
-		maxcount = num_refs;
+	if (!maxcount || ref_cbdata.array.nr < maxcount)
+		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref(refs[i], format, quote_style);
+		show_ref(ref_cbdata.array.items[i], format, quote_style);
 	return 0;
 }
Signature · 1 line
2.4.2
#13 lore raw

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

builtin/for-each-ref.c struct ref_array_item { · line 54

Quoted 4 lines · Karthik Nayak
builtin/for-each-ref.c +4 -0
+struct ref_filter_cbdata {
+	struct ref_array array;
+	struct ref_filter filter;
+};

I didn't notice this at first, but why introduce the structure like this when you are going to turn it into pointers later in PATCH 7:

Quoted 11 lines · Karthik Nayak
ref-filter.h +2 -2
@@ -42,12 +45,17 @@ struct ref_filter {
 };
 
 struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
+	struct ref_array *array;
+	struct ref_filter *filter;
 };
Signature · 2 lines
Matthieu Moy
http://www-verimag.imag.fr/~moy/
#14 lore raw

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

Quoted 9 lines · Matthieu Moy

builtin/for-each-ref.c struct ref_array_item { · line 54

Quoted 4 lines · Karthik Nayak
builtin/for-each-ref.c +4 -0
+struct ref_filter_cbdata {
+    struct ref_array array;
+    struct ref_filter filter;
+};

I didn't notice this at first, but why introduce the structure like this when you are going to turn it into pointers later in PATCH 7:

Here it is serving to for-each-ref, so I kept it this way so as to ensure that currently as per this patch

struct ref_filter_cbdata ref_cbdata;

would be the only declaration needed in for-each-ref.c If I made them pointers here I would need to have

struct ref_filter_cbdata ref_cbdata;
struct ref_filter filter;
struct ref_array array;
ref_cbdata.filter = &filter;
ref_cbdata.array = &array;

And this seemed like an overhead.
If you still think I need to change the code to reflect the change early on I will gladly do a re-roll.

Signature · 2 lines
Regards,
Karthik
#15 lore raw

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

Quoted 15 lines · Karthik Nayak
Quoted 9 lines · Matthieu Moy

builtin/for-each-ref.c struct ref_array_item { · line 54

Quoted 4 lines · Karthik Nayak
builtin/for-each-ref.c +4 -0
+struct ref_filter_cbdata {
+    struct ref_array array;
+    struct ref_filter filter;
+};

I didn't notice this at first, but why introduce the structure like this when you are going to turn it into pointers later in PATCH 7:

Here it is serving to for-each-ref, so I kept it this way so as to ensure that currently as per this patch

struct ref_filter_cbdata ref_cbdata;

Quoted 8 lines

would be the only declaration needed in for-each-ref.c If I made them pointers here I would need to have

struct ref_filter_cbdata ref_cbdata;
struct ref_filter filter;
struct ref_array array;
ref_cbdata.filter = &filter;
ref_cbdata.array = &array;

... but after PATCH 7, filter and array are passed to ref_filter so you don't have this overhead anyway. Makes sense.

Signature · 2 lines
Matthieu Moy
http://www-verimag.imag.fr/~moy/
#16 lore raw

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

Quoted 20 lines · Matthieu Moy
Quoted 18 lines · Karthik Nayak
Quoted 9 lines · Matthieu Moy

builtin/for-each-ref.c struct ref_array_item { · line 54

Quoted 4 lines · Karthik Nayak
builtin/for-each-ref.c +4 -0
+struct ref_filter_cbdata {
+    struct ref_array array;
+    struct ref_filter filter;
+};

I didn't notice this at first, but why introduce the structure like this when you are going to turn it into pointers later in PATCH 7:

Here it is serving to for-each-ref, so I kept it this way so as to ensure that currently as per this patch

struct ref_filter_cbdata ref_cbdata;

would be the only declaration needed in for-each-ref.c If I made them pointers here I would need to have

Quoted 9 lines
Quoted 5 lines

struct ref_filter_cbdata ref_cbdata;
struct ref_filter filter;
struct ref_array array;
ref_cbdata.filter = &filter;
ref_cbdata.array = &array;

... but after PATCH 7, filter and array are passed to ref_filter so you don't have this overhead anyway. Makes sense.

Yes, there we wouldn't have a ref_cbdata in 'for-each-ref'.
But this would be taken care of in 'filter_refs()'.
"Makes sense." Not sure if you're agreeing with me or you want me to re-roll.

Signature · 2 lines
Regards,
Karthik
#17 lore raw

Re: [PATCH v7 05/12] for-each-ref: introduce new structures for better organisation

Quoted 7 lines · Karthik Nayak
Quoted 3 lines

... but after PATCH 7, filter and array are passed to ref_filter so you don't have this overhead anyway. Makes sense.

Yes, there we wouldn't have a ref_cbdata in 'for-each-ref'. But this would be taken care of in 'filter_refs()'. "Makes sense." Not sure if you're agreeing with me or you want me to re-roll.

I'm agreeing with you.

Signature · 2 lines
Matthieu Moy
http://www-verimag.imag.fr/~moy/
#18 lore raw

PATCH v7 6/12 for-each-ref: introduce 'ref_array_clear()'

In reply to Karthik Nayak

Introduce and implement 'ref_array_clear()' which will free all allocated memory for 'ref_array'.

---
 builtin/for-each-ref.c | 21 +++++++++++++++++++++
 1 file changed, 21 insertions(+)
builtin/for-each-ref.c +21 -0
index 913d757..58396b2 100644
@@ -927,6 +927,26 @@ static int grab_single_ref(const char *refname, const struct object_id *oid,
 	return 0;
 }
 
+/*  Free memory allocated for a ref_array_item */
+static void free_array_item(struct ref_array_item *item)
+{
+	free((char *)item->symref);
+	free(item->refname);
+	free(item);
+}
+
+/* Free all memory allocated for ref_array */
+void ref_array_clear(struct ref_array *array)
+{
+	int i;
+
+	for (i = 0; i < array->nr; i++)
+		free_array_item(array->items[i]);
+	free(array->items);
+	array->items = NULL;
+	array->nr = array->alloc = 0;
+}
+
 static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
@@ -1157,5 +1177,6 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
 		show_ref(ref_cbdata.array.items[i], format, quote_style);
+	ref_array_clear(&ref_cbdata.array);
 	return 0;
 }
Signature · 1 line
2.4.2
#19 lore raw

PATCH v7 7/12 for-each-ref: rename some functions and make them public

In reply to Karthik Nayak

Rename some of the functions and make them publicly available. This is a preparatory step for moving code from 'for-each-ref' to 'ref-filter' to make meaningful, targeted services available to other commands via public APIs.

Functions renamed are:
parse_atom() -> parse_ref_filter_atom()
verify_format() -> verify_ref_format()
get_value() -> get_ref_atom_value()
grab_single_ref() -> ref_filter_handler()
sort_refs() -> ref_array_sort()
show_ref() -> show_ref_array_item()
default_sort() -> ref_default_sorting()
opt_parse_sort() -> parse_opt_ref_sorting()
cmp_ref_sort() -> cmp_ref_sorting()

Rename 'struct ref_sort' to 'struct ref_sorting' in this context.

---
 builtin/for-each-ref.c | 68 +++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)
builtin/for-each-ref.c +34 -34
index 58396b2..4a6eaba 100644
@@ -25,8 +25,8 @@ struct atom_value {
 	unsigned long ul; /* used for sorting when not FIELD_STR */
 };
 
-struct ref_sort {
-	struct ref_sort *next;
+struct ref_sorting {
+	struct ref_sorting *next;
 	int atom; /* index into 'struct atom_value *' array */
 	unsigned reverse : 1;
 };
@@ -113,7 +113,7 @@ static int need_color_reset_at_eol;
 /*
  * Used to parse format string and sort specifiers
  */
-static int parse_atom(const char *atom, const char *ep)
+int parse_ref_filter_atom(const char *atom, const char *ep)
 {
 	const char *sp;
 	int i, at;
@@ -190,7 +190,7 @@ static const char *find_next(const char *cp)
  * Make sure the format string is well formed, and parse out
  * the used atoms.
  */
-static int verify_format(const char *format)
+int verify_ref_format(const char *format)
 {
 	const char *cp, *sp;
 
@@ -202,7 +202,7 @@ static int verify_format(const char *format)
 		if (!ep)
 			return error("malformed format string %s", sp);
 		/* sp points at "%(" and ep points at the closing ")" */
-		at = parse_atom(sp + 2, ep);
+		at = parse_ref_filter_atom(sp + 2, ep);
 		cp = ep + 1;
 
 		if (skip_prefix(used_atom[at], "color:", &color))
@@ -409,7 +409,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	/*
 	 * We got here because atomname ends in "date" or "date<something>";
 	 * it's not possible that <something> is not ":<format>" because
-	 * parse_atom() wouldn't have allowed it, so we can assume that no
+	 * parse_ref_filter_atom() wouldn't have allowed it, so we can assume that no
 	 * ":" means no format is specified, and use the default.
 	 */
 	formatp = strchr(atomname, ':');
@@ -849,7 +849,7 @@ static void populate_value(struct ref_array_item *ref)
  * Given a ref, return the value for the atom.  This lazily gets value
  * out of the object by calling populate value.
  */
-static void get_value(struct ref_array_item *ref, int atom, struct atom_value **v)
+static void get_ref_atom_value(struct ref_array_item *ref, int atom, struct atom_value **v)
 {
 	if (!ref->value) {
 		populate_value(ref);
@@ -900,8 +900,7 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
-static int grab_single_ref(const char *refname, const struct object_id *oid,
-			   int flag, void *cb_data)
+int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
 	struct ref_filter *filter = &ref_cbdata->filter;
@@ -947,14 +946,14 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
-static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref_array_item *b)
+static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
 	int cmp;
 	cmp_type cmp_type = used_atom_type[s->atom];
 
-	get_value(a, s->atom, &va);
-	get_value(b, s->atom, &vb);
+	get_ref_atom_value(a, s->atom, &va);
+	get_ref_atom_value(b, s->atom, &vb);
 	switch (cmp_type) {
 	case FIELD_STR:
 		cmp = strcmp(va->s, vb->s);
@@ -971,24 +970,24 @@ static int cmp_ref_sort(struct ref_sort *s, struct ref_array_item *a, struct ref
 	return (s->reverse) ? -cmp : cmp;
 }
 
-static struct ref_sort *ref_sort;
+static struct ref_sorting *ref_sorting;
 static int compare_refs(const void *a_, const void *b_)
 {
 	struct ref_array_item *a = *((struct ref_array_item **)a_);
 	struct ref_array_item *b = *((struct ref_array_item **)b_);
-	struct ref_sort *s;
+	struct ref_sorting *s;
 
-	for (s = ref_sort; s; s = s->next) {
-		int cmp = cmp_ref_sort(s, a, b);
+	for (s = ref_sorting; s; s = s->next) {
+		int cmp = cmp_ref_sorting(s, a, b);
 		if (cmp)
 			return cmp;
 	}
 	return 0;
 }
 
-static void sort_refs(struct ref_sort *sort, struct ref_array *array)
+void ref_array_sort(struct ref_sorting *sort, struct ref_array *array)
 {
-	ref_sort = sort;
+	ref_sorting = sort;
 	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
@@ -1056,7 +1055,7 @@ static void emit(const char *cp, const char *ep)
 	}
 }
 
-static void show_ref(struct ref_array_item *info, const char *format, int quote_style)
+void show_ref_array_item(struct ref_array_item *info, const char *format, int quote_style)
 {
 	const char *cp, *sp, *ep;
 
@@ -1066,7 +1065,7 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
 		ep = strchr(sp, ')');
 		if (cp < sp)
 			emit(cp, sp);
-		get_value(info, parse_atom(sp + 2, ep), &atomv);
+		get_ref_atom_value(info, parse_ref_filter_atom(sp + 2, ep), &atomv);
 		print_value(atomv, quote_style);
 	}
 	if (*cp) {
@@ -1085,21 +1084,22 @@ static void show_ref(struct ref_array_item *info, const char *format, int quote_
 	putchar('\n');
 }
 
-static struct ref_sort *default_sort(void)
+/*  If no sorting option is given, use refname to sort as default */
+struct ref_sorting *ref_default_sorting(void)
 {
 	static const char cstr_name[] = "refname";
 
-	struct ref_sort *sort = xcalloc(1, sizeof(*sort));
+	struct ref_sorting *sort = xcalloc(1, sizeof(*sort));
 
 	sort->next = NULL;
-	sort->atom = parse_atom(cstr_name, cstr_name + strlen(cstr_name));
+	sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
 	return sort;
 }
 
-static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
+int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 {
-	struct ref_sort **sort_tail = opt->value;
-	struct ref_sort *s;
+	struct ref_sorting **sort_tail = opt->value;
+	struct ref_sorting *s;
 	int len;
 
 	if (!arg) /* should --no-sort void the list ? */
@@ -1114,7 +1114,7 @@ static int opt_parse_sort(const struct option *opt, const char *arg, int unset)
 		arg++;
 	}
 	len = strlen(arg);
-	s->atom = parse_atom(arg, arg+len);
+	s->atom = parse_ref_filter_atom(arg, arg+len);
 	return 0;
 }
 
@@ -1127,7 +1127,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
-	struct ref_sort *sort = NULL, **sort_tail = &sort;
+	struct ref_sorting *sort = NULL, **sort_tail = &sort;
 	int maxcount = 0, quote_style = 0;
 	struct ref_filter_cbdata ref_cbdata;
 
@@ -1145,7 +1145,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
 		OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
-			    N_("field name to sort on"), &opt_parse_sort),
+			    N_("field name to sort on"), &parse_opt_ref_sorting),
 		OPT_END(),
 	};
 
@@ -1158,25 +1158,25 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		error("more than one quoting style?");
 		usage_with_options(for_each_ref_usage, opts);
 	}
-	if (verify_format(format))
+	if (verify_ref_format(format))
 		usage_with_options(for_each_ref_usage, opts);
 
 	if (!sort)
-		sort = default_sort();
+		sort = ref_default_sorting();
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
 	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
 	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(grab_single_ref, &ref_cbdata);
+	for_each_rawref(ref_filter_handler, &ref_cbdata);
 
-	sort_refs(sort, &ref_cbdata.array);
+	ref_array_sort(sort, &ref_cbdata.array);
 
 	if (!maxcount || ref_cbdata.array.nr < maxcount)
 		maxcount = ref_cbdata.array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref(ref_cbdata.array.items[i], format, quote_style);
+		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
 	ref_array_clear(&ref_cbdata.array);
 	return 0;
 }
Signature · 1 line
2.4.2
#20 lore raw

PATCH v7 8/12 for-each-ref: rename variables called sort to sorting

In reply to Karthik Nayak

Rename all the variables called sort to sorting to match the function/structure name changes made in the previous patch.

---
 builtin/for-each-ref.c | 28 ++++++++++++++--------------
 1 file changed, 14 insertions(+), 14 deletions(-)
builtin/for-each-ref.c +14 -14
index 4a6eaba..3cf456a 100644
@@ -985,9 +985,9 @@ static int compare_refs(const void *a_, const void *b_)
 	return 0;
 }
 
-void ref_array_sort(struct ref_sorting *sort, struct ref_array *array)
+void ref_array_sort(struct ref_sorting *sorting, struct ref_array *array)
 {
-	ref_sorting = sort;
+	ref_sorting = sorting;
 	qsort(array->items, array->nr, sizeof(struct ref_array_item *), compare_refs);
 }
 
@@ -1089,16 +1089,16 @@ struct ref_sorting *ref_default_sorting(void)
 {
 	static const char cstr_name[] = "refname";
 
-	struct ref_sorting *sort = xcalloc(1, sizeof(*sort));
+	struct ref_sorting *sorting = xcalloc(1, sizeof(*sorting));
 
-	sort->next = NULL;
-	sort->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
-	return sort;
+	sorting->next = NULL;
+	sorting->atom = parse_ref_filter_atom(cstr_name, cstr_name + strlen(cstr_name));
+	return sorting;
 }
 
 int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 {
-	struct ref_sorting **sort_tail = opt->value;
+	struct ref_sorting **sorting_tail = opt->value;
 	struct ref_sorting *s;
 	int len;
 
@@ -1106,8 +1106,8 @@ int parse_opt_ref_sorting(const struct option *opt, const char *arg, int unset)
 		return -1;
 
 	s = xcalloc(1, sizeof(*s));
-	s->next = *sort_tail;
-	*sort_tail = s;
+	s->next = *sorting_tail;
+	*sorting_tail = s;
 
 	if (*arg == '-') {
 		s->reverse = 1;
@@ -1127,7 +1127,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 {
 	int i;
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
-	struct ref_sorting *sort = NULL, **sort_tail = &sort;
+	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
 	struct ref_filter_cbdata ref_cbdata;
 
@@ -1144,7 +1144,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 		OPT_GROUP(""),
 		OPT_INTEGER( 0 , "count", &maxcount, N_("show only <n> matched refs")),
 		OPT_STRING(  0 , "format", &format, N_("format"), N_("format to use for the output")),
-		OPT_CALLBACK(0 , "sort", sort_tail, N_("key"),
+		OPT_CALLBACK(0 , "sort", sorting_tail, N_("key"),
 			    N_("field name to sort on"), &parse_opt_ref_sorting),
 		OPT_END(),
 	};
@@ -1161,8 +1161,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	if (verify_ref_format(format))
 		usage_with_options(for_each_ref_usage, opts);
 
-	if (!sort)
-		sort = ref_default_sorting();
+	if (!sorting)
+		sorting = ref_default_sorting();
 
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
@@ -1171,7 +1171,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	ref_cbdata.filter.name_patterns = argv;
 	for_each_rawref(ref_filter_handler, &ref_cbdata);
 
-	ref_array_sort(sort, &ref_cbdata.array);
+	ref_array_sort(sorting, &ref_cbdata.array);
 
 	if (!maxcount || ref_cbdata.array.nr < maxcount)
 		maxcount = ref_cbdata.array.nr;
Signature · 1 line
2.4.2
#21 lore raw

PATCH v7 9/12 ref-filter: add 'ref-filter.h'

In reply to Karthik Nayak

This is step one of creating a common library for 'for-each-ref', 'branch -l' and 'tag -l'. This creates a header file with the functions and data structures that ref-filter will provide. We move the data structures created in for-each-ref to this header file.

---
 builtin/for-each-ref.c | 41 +------------------------------
 ref-filter.h           | 66 ++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 67 insertions(+), 40 deletions(-)
 create mode 100644 ref-filter.h
builtin/for-each-ref.c +1 -40
index 3cf456a..2fbbc59 100644
@@ -10,49 +10,10 @@
 #include "parse-options.h"
 #include "remote.h"
 #include "color.h"
-
-/* Quoting styles */
-#define QUOTE_NONE 0
-#define QUOTE_SHELL 1
-#define QUOTE_PERL 2
-#define QUOTE_PYTHON 4
-#define QUOTE_TCL 8
+#include "ref-filter.h"
 
 typedef enum { FIELD_STR, FIELD_ULONG, FIELD_TIME } cmp_type;
 
-struct atom_value {
-	const char *s;
-	unsigned long ul; /* used for sorting when not FIELD_STR */
-};
-
-struct ref_sorting {
-	struct ref_sorting *next;
-	int atom; /* index into 'struct atom_value *' array */
-	unsigned reverse : 1;
-};
-
-struct ref_array_item {
-	unsigned char objectname[20];
-	int flag;
-	const char *symref;
-	struct atom_value *value;
-	char *refname;
-};
-
-struct ref_array {
-	int nr, alloc;
-	struct ref_array_item **items;
-};
-
-struct ref_filter {
-	const char **name_patterns;
-};
-
-struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
-};
-
 static struct {
 	const char *name;
 	cmp_type cmp_type;
ref-filter.h new file +66 -0

66 lines not on this page · raw message

Signature · 1 line
2.4.2
#22 lore raw

PATCH v7 10/12 ref-filter: move code from 'for-each-ref'

In reply to Karthik Nayak

Move most of the code from 'for-each-ref' to 'ref-filter' to make it publicly available to other commands, this is to unify the code of 'tag -l', 'branch -l' and 'for-each-ref' so that they can share their implementations with each other.

Add 'ref-filter' to the Makefile, this completes the movement of code from 'for-each-ref' to 'ref-filter'.

---
 Makefile               |    1 +
 builtin/for-each-ref.c | 1074 -----------------------------------------------
 ref-filter.c           | 1078 ++++++++++++++++++++++++++++++++++++++++++++++++
 3 files changed, 1079 insertions(+), 1074 deletions(-)
 create mode 100644 ref-filter.c
Makefile +1 -0
index 54ec511..d715b66 100644
@@ -762,6 +762,7 @@ LIB_OBJS += reachable.o
 LIB_OBJS += read-cache.o
 LIB_OBJS += reflog-walk.o
 LIB_OBJS += refs.o
+LIB_OBJS += ref-filter.o
 LIB_OBJS += remote.o
 LIB_OBJS += replace_object.o
 LIB_OBJS += rerere.o
builtin/for-each-ref.c +0 -1074

1,083 lines not on this page · raw message

ref-filter.c new file +1078 -0

1,078 lines not on this page · raw message

Signature · 1 line
2.4.2
#23 lore raw

PATCH v7 11/12 for-each-ref: introduce filter_refs()

In reply to Karthik Nayak

Introduce filter_refs() which will act as an API for filtering a set of refs. Based on the type of refs the user has requested, we iterate through those refs and apply filters as per the given ref_filter structure and finally store the filtered refs in the ref_array structure.

Currently this will wrap around ref_filter_handler(). Hence, ref_filter_handler is made file scope static.

As users of this API will no longer send a ref_filter_cbdata structure directly, we make the elements of ref_filter_cbdata pointers. We can now use the information given by the users to obtain our own ref_filter_cbdata structure. Changes are made to support the change in ref_filter_cbdata structure.

Make 'for-each-ref' use this API.

---
 builtin/for-each-ref.c | 21 +++++++++++----------
 ref-filter.c           | 30 ++++++++++++++++++++++++++----
 ref-filter.h           | 16 ++++++++++++----
 3 files changed, 49 insertions(+), 18 deletions(-)
builtin/for-each-ref.c +11 -10
index 637fc4a..e2f15e6 100644
@@ -16,7 +16,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
-	struct ref_filter_cbdata ref_cbdata;
+	struct ref_array array;
+	struct ref_filter filter;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -54,16 +55,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
-	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(ref_filter_handler, &ref_cbdata);
+	memset(&array, 0, sizeof(array));
+	memset(&filter, 0, sizeof(filter));
+	filter.name_patterns = argv;
+	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);
+	ref_array_sort(sorting, &array);
 
-	ref_array_sort(sorting, &ref_cbdata.array);
-
-	if (!maxcount || ref_cbdata.array.nr < maxcount)
-		maxcount = ref_cbdata.array.nr;
+	if (!maxcount || array.nr < maxcount)
+		maxcount = array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
-	ref_array_clear(&ref_cbdata.array);
+		show_ref_array_item(array.items[i], format, quote_style);
+	ref_array_clear(&array);
 	return 0;
 }
ref-filter.c +26 -4
index 74fe295..6f7defc 100644
@@ -859,10 +859,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
-	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_filter *filter = ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -880,8 +880,8 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
-	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
+	ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
 	return 0;
 }
 
@@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter)
+{
+	struct ref_filter_cbdata ref_cbdata;
+
+	ref_cbdata.array = array;
+	ref_cbdata.filter = filter;
+
+	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
+		return for_each_rawref(ref_filter_handler, &ref_cbdata);
+	else if (type & FILTER_REFS_ALL)
+		return for_each_ref(ref_filter_handler, &ref_cbdata);
+	else
+		die("filter_refs: invalid type");
+	return 0;
+}
+
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
ref-filter.h +12 -4
index d9e043f..f917ebc 100644
@@ -13,6 +13,9 @@
 #define QUOTE_PYTHON 4
 #define QUOTE_TCL 8
 
+#define FILTER_REFS_INCLUDE_BROKEN 0x1
+#define FILTER_REFS_ALL 0x2
+
 struct atom_value {
 	const char *s;
 	unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -42,12 +45,17 @@ struct ref_filter {
 };
 
 struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
+	struct ref_array *array;
+	struct ref_filter *filter;
 };
 
-/*  Callback function for for_each_*ref(). This filters the refs based on the filters set */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data);
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
 /*  Parse format string and sort specifiers */
Signature · 1 line
2.4.2
#24 lore raw

PATCH v7 11/12 for-each-ref: introduce filter_refs()

Introduce filter_refs() which will act as an API for filtering a set of refs. Based on the type of refs the user has requested, we iterate through those refs and apply filters as per the given ref_filter structure and finally store the filtered refs in the ref_array structure.

Currently this will wrap around ref_filter_handler(). Hence, ref_filter_handler is made file scope static.

As users of this API will no longer send a ref_filter_cbdata structure directly, we make the elements of ref_filter_cbdata pointers. We can now use the information given by the users to obtain our own ref_filter_cbdata structure. Changes are made to support the change in ref_filter_cbdata structure.

Make 'for-each-ref' use this API.

---
 builtin/for-each-ref.c | 21 +++++++++++----------
 ref-filter.c           | 30 ++++++++++++++++++++++++++----
 ref-filter.h           | 16 ++++++++++++----
 3 files changed, 49 insertions(+), 18 deletions(-)
builtin/for-each-ref.c +11 -10
index 637fc4a..7919206 100644
@@ -16,7 +16,8 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	const char *format = "%(objectname) %(objecttype)\t%(refname)";
 	struct ref_sorting *sorting = NULL, **sorting_tail = &sorting;
 	int maxcount = 0, quote_style = 0;
-	struct ref_filter_cbdata ref_cbdata;
+	struct ref_array array;
+	struct ref_filter filter;
 
 	struct option opts[] = {
 		OPT_BIT('s', "shell", &quote_style,
@@ -54,16 +55,16 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 	/* for warn_ambiguous_refs */
 	git_config(git_default_config, NULL);
 
-	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
-	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(ref_filter_handler, &ref_cbdata);
+	memset(&array, 0, sizeof(array));
+	memset(&filter, 0, sizeof(filter));
+	filter.name_patterns = argv;
+	filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);
+	ref_array_sort(sorting, &array);
 
-	ref_array_sort(sorting, &ref_cbdata.array);
-
-	if (!maxcount || ref_cbdata.array.nr < maxcount)
-		maxcount = ref_cbdata.array.nr;
+	if (!maxcount || array.nr < maxcount)
+		maxcount = array.nr;
 	for (i = 0; i < maxcount; i++)
-		show_ref_array_item(ref_cbdata.array.items[i], format, quote_style);
-	ref_array_clear(&ref_cbdata.array);
+		show_ref_array_item(array.items[i], format, quote_style);
+	ref_array_clear(&array);
 	return 0;
 }
ref-filter.c +26 -4
index 74fe295..9371091 100644
@@ -859,10 +859,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
  * A call-back given to for_each_ref().  Filter refs and keep them for
  * later object processing.
  */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
+static int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data)
 {
 	struct ref_filter_cbdata *ref_cbdata = cb_data;
-	struct ref_filter *filter = &ref_cbdata->filter;
+	struct ref_filter *filter = ref_cbdata->filter;
 	struct ref_array_item *ref;
 
 	if (flag & REF_BAD_NAME) {
@@ -880,8 +880,8 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 	 */
 	ref = new_ref_array_item(refname, oid->hash, flag);
 
-	REALLOC_ARRAY(ref_cbdata->array.items, ref_cbdata->array.nr + 1);
-	ref_cbdata->array.items[ref_cbdata->array.nr++] = ref;
+	REALLOC_ARRAY(ref_cbdata->array->items, ref_cbdata->array->nr + 1);
+	ref_cbdata->array->items[ref_cbdata->array->nr++] = ref;
 	return 0;
 }
 
@@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type)
+{
+	struct ref_filter_cbdata ref_cbdata;
+
+	ref_cbdata.array = array;
+	ref_cbdata.filter = filter;
+
+	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
+		return for_each_rawref(ref_filter_handler, &ref_cbdata);
+	else if (type & FILTER_REFS_ALL)
+		return for_each_ref(ref_filter_handler, &ref_cbdata);
+	else
+		die("filter_refs: invalid type");
+	return 0;
+}
+
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;
ref-filter.h +12 -4
index d9e043f..3999340 100644
@@ -13,6 +13,9 @@
 #define QUOTE_PYTHON 4
 #define QUOTE_TCL 8
 
+#define FILTER_REFS_INCLUDE_BROKEN 0x1
+#define FILTER_REFS_ALL 0x2
+
 struct atom_value {
 	const char *s;
 	unsigned long ul; /* used for sorting when not FIELD_STR */
@@ -42,12 +45,17 @@ struct ref_filter {
 };
 
 struct ref_filter_cbdata {
-	struct ref_array array;
-	struct ref_filter filter;
+	struct ref_array *array;
+	struct ref_filter *filter;
 };
 
-/*  Callback function for for_each_*ref(). This filters the refs based on the filters set */
-int ref_filter_handler(const char *refname, const struct object_id *oid, int flag, void *cb_data);
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, struct ref_filter *filter, unsigned int type);
 /*  Clear all memory allocated to ref_array */
 void ref_array_clear(struct ref_array *array);
 /*  Parse format string and sort specifiers */
Signature · 1 line
2.4.2
#25 lore raw

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

In reply to Karthik Nayak

builtin/for-each-ref.c int cmd_for_each_ref(int argc, const char **argv, const char *prefix) · line 61

Quoted 1 line · Karthik Nayak
builtin/for-each-ref.c +1 -0
+	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);

I think it is more common to have options at the end, so I'd write it as

filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);

(changing the declaration too, obviously)

I really like the way cmd_for_each_ref looks like now.

Quoted 15 lines
ref-filter.c +11 -0
@@ -905,6 +905,28 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. Based on the type of refs the user
+ * has requested, we iterate through those refs and apply filters
+ * as per the given ref_filter structure and finally store the
+ * filtered refs in the ref_array structure.
+ */
+int filter_refs(struct ref_array *array, unsigned int type, struct ref_filter *filter)
+{
+	struct ref_filter_cbdata ref_cbdata;
+
+	ref_cbdata.array = array;

ref-filter.c void ref_array_clear(struct ref_array *array) · line 928

Quoted 10 lines
ref-filter.c +10 -0
+	ref_cbdata.filter = filter;
+
+	if (type & (FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN))
+		return for_each_rawref(ref_filter_handler, &ref_cbdata);
+	else if (type & FILTER_REFS_ALL)
+		return for_each_ref(ref_filter_handler, &ref_cbdata);
+	else
+		die("filter_refs: invalid type");
+	return 0;
+}

I thought you would make a helper function that would return a pointer to either for_each_rawref or for_each_ref (or another later), but that would probably be overkill.

Signature · 2 lines
Matthieu Moy
http://www-verimag.imag.fr/~moy/
#26 lore raw

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

Quoted 9 lines · Matthieu Moy

I think it is more common to have options at the end, so I'd write it as

filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);

(changing the declaration too, obviously)

I really like the way cmd_for_each_ref looks like now.

Ok! will reply to this mail with a small changed patch!

Quoted 5 lines

I thought you would make a helper function that would return a pointer to either for_each_rawref or for_each_ref (or another later), but that would probably be overkill.

Yea that does seem like an overkill!

Signature · 2 lines
Regards,
Karthik
#27 lore raw

Re: [PATCH v7 11/12] for-each-ref: introduce filter_refs()

In reply to Matthieu Moy
Quoted 9 lines · Matthieu Moy

builtin/for-each-ref.c int cmd_for_each_ref(int argc, const char **argv, const char *prefix) · line 61

Quoted 1 line · Karthik Nayak
builtin/for-each-ref.c +1 -0
+	filter_refs(&array, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN, &filter);

I think it is more common to have options at the end, so I'd write it as

filter_refs(&array, &filter, FILTER_REFS_ALL | FILTER_REFS_INCLUDE_BROKEN);

(changing the declaration too, obviously)

Good point.

Quoted 2 lines

I really like the way cmd_for_each_ref looks like now.

Likewise.

#28 lore raw

PATCH v7 12/12 ref-filter: make 'ref_array_item' use a FLEX_ARRAY for refname

In reply to Karthik Nayak

This would remove the need of using a pointer to store refname.

---
 ref-filter.c | 7 ++++---
 ref-filter.h | 2 +-
 2 files changed, 5 insertions(+), 4 deletions(-)
ref-filter.c +4 -3
index 6f7defc..310ecd6 100644
@@ -847,8 +847,10 @@ static struct ref_array_item *new_ref_array_item(const char *refname,
 						 const unsigned char *objectname,
 						 int flag)
 {
-	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item));
-	ref->refname = xstrdup(refname);
+	size_t len = strlen(refname);
+	struct ref_array_item *ref = xcalloc(1, sizeof(struct ref_array_item) + len + 1);
+	memcpy(ref->refname, refname, len);
+	ref->refname[len] = '\0';
 	hashcpy(ref->objectname, objectname);
 	ref->flag = flag;
 
@@ -889,7 +891,6 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
 static void free_array_item(struct ref_array_item *item)
 {
 	free((char *)item->symref);
-	free(item->refname);
 	free(item);
 }
 
ref-filter.h +1 -1
index f917ebc..6ab2a75 100644
@@ -32,7 +32,7 @@ struct ref_array_item {
 	int flag;
 	const char *symref;
 	struct atom_value *value;
-	char *refname;
+	char refname[FLEX_ARRAY];
 };
 
 struct ref_array {
Signature · 1 line
2.4.2
#29 lore raw

Re: [PATCH v7 01/12] for-each-ref: extract helper functions out of grab_single_ref()

In reply to Karthik Nayak
Quoted 25 lines · Karthik Nayak

Extract two helper functions out of grab_single_ref(). Firstly, new_refinfo() which is used to allocate memory for a new refinfo structure and copy the objectname, refname and flag to it. Secondly, match_name_as_path() which when given an array of patterns and the refname checks if the refname matches any of the patterns given while the pattern is a pathname, also supports wildcard characters.

This is a preperatory patch for restructuring 'for-each-ref' and eventually moving most of it to 'ref-filter' to provide the functionality to similar commands via public API's.

---
 builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 41 insertions(+), 23 deletions(-)
builtin/for-each-ref.c +0 -0
index f7e51a7..67c8b62 100644

builtin/for-each-ref.c struct grab_ref_cbdata { · line 854

Quoted 4 lines
builtin/for-each-ref.c +1 -0
@@ -851,6 +851,44 @@ struct grab_ref_cbdata {
 };
 
 /*
+ * Return 1 if the refname matches with one of the patterns,

s/with //;

builtin/for-each-ref.c struct grab_ref_cbdata { · line 858

Quoted 4 lines
builtin/for-each-ref.c +4 -0
+ * otherwise 0.  The patterns can be literal prefix (e.g. a
+ * refname "refs/heads/master" matches a pattern "refs/heads/")
+ * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
+ */

I know this was my bad suggestion, but "refs/heads/m" can be thought of as a "literal prefix" that may match "refs/heads/master"; we do not want to make that match, so perhaps "literal" is a bad way to say this. "A pattern can be a path prefix or a worldcard"?

#30 lore raw

Re: [PATCH v7 01/12] for-each-ref: extract helper functions out of grab_single_ref()

Quoted 29 lines · Junio C Hamano
Quoted 27 lines · Karthik Nayak

Extract two helper functions out of grab_single_ref(). Firstly, new_refinfo() which is used to allocate memory for a new refinfo structure and copy the objectname, refname and flag to it. Secondly, match_name_as_path() which when given an array of patterns and the refname checks if the refname matches any of the patterns given while the pattern is a pathname, also supports wildcard characters.

This is a preperatory patch for restructuring 'for-each-ref' and eventually moving most of it to 'ref-filter' to provide the functionality to similar commands via public API's.

---
  builtin/for-each-ref.c | 64 ++++++++++++++++++++++++++++++++------------------
  1 file changed, 41 insertions(+), 23 deletions(-)
builtin/for-each-ref.c +0 -0
index f7e51a7..67c8b62 100644
@@ -851,6 +851,44 @@ struct grab_ref_cbdata {
  };
 
Quoted 4 lines

builtin/for-each-ref.c struct grab_ref_cbdata { · line 854

Quoted 2 lines
builtin/for-each-ref.c +1 -0
  /*
+ * Return 1 if the refname matches with one of the patterns,

s/with //;

Thanks! will change :)

Quoted 11 lines

builtin/for-each-ref.c struct grab_ref_cbdata { · line 858

Quoted 5 lines
builtin/for-each-ref.c +4 -0
+ * otherwise 0.  The patterns can be literal prefix (e.g. a
+ * refname "refs/heads/master" matches a pattern "refs/heads/")
+ * or a wildcard (e.g. the same ref matches "refs/heads/m*",too).
+ */

I know this was my bad suggestion, but "refs/heads/m" can be thought of as a "literal prefix" that may match "refs/heads/master"; we do not want to make that match, so perhaps "literal" is a bad way to say this. "A pattern can be a path prefix or a worldcard"?

Yes! that sounds right, after all its doing a path match.

Signature · 2 lines
Regards,
Karthik
#31 lore raw
In reply to Karthik Nayak
Quoted 7 lines · Karthik Nayak

The previous version of this patch can be found here:
http://thread.gmane.org/gmane.comp.version-control.git/270922

Changes found in this version:

*    Various changes to the 'filter_refs()' function.
*    Split 'for-each-ref: clean up code' into two commits.
*    Other small changes.

My previous comments are all taken into account. The whole series looks good to me now.

Thanks,

Signature · 2 lines
Matthieu Moy
http://www-verimag.imag.fr/~moy/