create ref-filter from for-each-ref

---
 builtin/for-each-ref.c | 63 ++++++++++++++++++++++++++++++++------------------
 1 file changed, 40 insertions(+), 23 deletions(-)

builtin/for-each-ref.c +40 ~~-23~~

index f7e51a7..f939fd3 100644
@@ -851,6 +851,43 @@ struct grab_ref_cbdata {
 };
 
 /*
+ * Given a refname, return 1 if the refname matches with one of the patterns
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
+ */
+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 +903,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

Karthik Nayak

author

2016-06-15 23:05 UTC #2 lore raw

PATCH v6 2/11 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.

Change comment in 'struct ref_sort' to reflect changes in code.

---
 builtin/for-each-ref.c | 9 +++------
 1 file changed, 3 insertions(+), 6 deletions(-)

builtin/for-each-ref.c +3 -6

index f939fd3..406de88 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;
 };

@@ -896,7 +896,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);

@@ -913,10 +912,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

Karthik Nayak

author

2016-06-15 23:05 UTC #3 lore raw

PATCH v6 3/11 for-each-ref: rename 'refinfo' to 'ref_array_item'

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.

Make 'symref' a non const char pointer, so that the compiler doesn't throw an error when we try to free the memory allocated to it.

---
 builtin/for-each-ref.c | 42 +++++++++++++++++++++---------------------
 1 file changed, 21 insertions(+), 21 deletions(-)

builtin/for-each-ref.c +21 ~~-21~~

index 406de88..763224c 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;
+	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;
 };

@@ -874,12 +874,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;

@@ -895,7 +895,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);

@@ -910,14 +910,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;

@@ -944,8 +944,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) {

@@ -956,10 +956,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)

@@ -1026,7 +1026,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;

@@ -1099,7 +1099,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

Karthik Nayak

author

2016-06-15 23:05 UTC #4 lore raw

PATCH v6 4/11 for-each-ref: introduce new structures for better organisation

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 763224c..42a89cd 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;
-};
-
 /*
  * Given a refname, return 1 if the refname matches with one of the patterns
  * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'

@@ -894,7 +902,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) {

@@ -902,7 +911,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;
 
 	/*

@@ -912,8 +921,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;
 }

@@ -956,10 +965,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)

@@ -1095,12 +1104,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,

@@ -1138,17 +1146,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

Karthik Nayak

author

2016-06-15 23:05 UTC #5 lore raw

PATCH v6 5/11 for-each-ref: introduce 'ref_array_clear()'

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 42a89cd..a65a4db 100644
@@ -926,6 +926,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(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;

@@ -1156,5 +1176,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

Karthik Nayak

author

2016-06-15 23:05 UTC #6 lore raw

PATCH v6 6/11 for-each-ref: rename some functions and make them public

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.

Based-on-patch-by Jeff King <[email protected]>

---
 builtin/for-each-ref.c | 68 +++++++++++++++++++++++++-------------------------
 1 file changed, 34 insertions(+), 34 deletions(-)

builtin/for-each-ref.c +34 ~~-34~~

index a65a4db..11552ad 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);

@@ -899,8 +899,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;

@@ -946,14 +945,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);

@@ -970,24 +969,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);
 }

@@ -1055,7 +1054,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;

@@ -1065,7 +1064,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) {

@@ -1084,21 +1083,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 ? */

@@ -1113,7 +1113,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;
 }

@@ -1126,7 +1126,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;

@@ -1144,7 +1144,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(),
 	};

@@ -1157,25 +1157,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

Karthik Nayak

author

2016-06-15 23:05 UTC #7 lore raw

PATCH v6 7/11 for-each-ref: rename variables called sort to sorting

Rename all the variables called sort to sorting to match the function 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 11552ad..c0085be 100644
@@ -984,9 +984,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);
 }

@@ -1088,16 +1088,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;

@@ -1105,8 +1105,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;

@@ -1126,7 +1126,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;

@@ -1143,7 +1143,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(),
 	};

@@ -1160,8 +1160,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);

@@ -1170,7 +1170,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

Karthik Nayak

author

2016-06-15 23:05 UTC #8 lore raw

PATCH v6 8/11 ref-filter: add 'ref-filter.h'

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 c0085be..063de00 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;
-	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

Karthik Nayak

author

2016-06-15 23:05 UTC #9 lore raw

PATCH v6 9/11 ref-filter: move code from 'for-each-ref'

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 | 1075 +----------------------------------------------
 ref-filter.c           | 1077 ++++++++++++++++++++++++++++++++++++++++++++++++
 ref-filter.h           |    8 +-
 4 files changed, 1085 insertions(+), 1076 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 +1 ~~-1074~~

1,090 lines not on this page · raw message

ref-filter.c new file +1077 -0

1,077 lines not on this page · raw message

ref-filter.h +6 -2

index c2c5d37..15e6766 100644
@@ -46,8 +46,12 @@ struct ref_filter_cbdata {
 	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. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data);
 /*  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

Karthik Nayak

author

2016-06-15 23:05 UTC #10 lore raw

PATCH v6 9/11 ref-filter: move code from 'for-each-ref'

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 | 1075 +----------------------------------------------
 ref-filter.c           | 1077 ++++++++++++++++++++++++++++++++++++++++++++++++
 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 +1 ~~-1074~~

1,090 lines not on this page · raw message

ref-filter.c new file +1077 -0

1,077 lines not on this page · raw message

Signature · 1 line

2.4.2

Matthieu Moy

2016-06-15 23:05 UTC #11 lore raw

Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'

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

Quoted 8 lines · Karthik Nayak

builtin/for-each-ref.c +1 -1

@@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 
 	memset(&ref_cbdata, 0, sizeof(ref_cbdata));
 	ref_cbdata.filter.name_patterns = argv;
-	for_each_rawref(ref_filter_handler, &ref_cbdata);
+	filter_refs(for_each_rawref, &ref_cbdata);

This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet!

Signature · 2 lines

Matthieu Moy
http://www-verimag.imag.fr/~moy/

Karthik Nayak

author

2016-06-15 23:05 UTC #12 lore raw

Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'

Quoted 6 lines · Matthieu Moy

Quoted 4 lines · Karthik Nayak
builtin/for-each-ref.c +0 -0
@@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
 

Quoted 8 lines

builtin/for-each-ref.c int cmd_for_each_ref(int argc, const char **argv, const char *prefix) · line 59
Quoted 4 lines
builtin/for-each-ref.c +1 -1
      memset(&ref_cbdata, 0, sizeof(ref_cbdata));
      ref_cbdata.filter.name_patterns = argv;
-    for_each_rawref(ref_filter_handler, &ref_cbdata);
+    filter_refs(for_each_rawref, &ref_cbdata);
This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet!

That definitely is, this happened after fixing merges I suppose. Ignore it for now, I'll fix it.

Signature · 2 lines

Regards,
Karthik

Junio C Hamano

2016-06-15 23:05 UTC #13 lore raw

Re: [PATCH v6 09/11] ref-filter: move code from 'for-each-ref'

Quoted 16 lines · Karthik Nayak

Quoted 7 lines · Matthieu Moy
Quoted 5 lines · Karthik Nayak
builtin/for-each-ref.c +0 -0
@@ -1129,7 +56,7 @@ int cmd_for_each_ref(int argc, const char
 **argv, const char *prefix)
 
Quoted 8 lines
builtin/for-each-ref.c int cmd_for_each_ref(int argc, const char **argv, const char *prefix) · line 59
Quoted 4 lines
builtin/for-each-ref.c +1 -1
      memset(&ref_cbdata, 0, sizeof(ref_cbdata));
      ref_cbdata.filter.name_patterns = argv;
-    for_each_rawref(ref_filter_handler, &ref_cbdata);
+    filter_refs(for_each_rawref, &ref_cbdata);
This seems unrelated from the rest of the patch. And you haven't introduced filter_refs yet!

Quoted 2 lines

That definitely is, this happened after fixing merges I suppose. Ignore it for now, I'll fix it.

I am not sure which one of two copies of 09/11 is being discussed, but I'll stop here for now. Most of the patches up to 08/11 looked sensible, if the renames were a bit too noisy.

Thanks.

Karthik Nayak

author

2016-06-15 23:05 UTC #14 lore raw

PATCH v6 10/11 for-each-ref: introduce filter_refs()

Introduce filter_refs() which will act as an API for users to call and provide a function which will iterate over a set of refs while filtering out the required refs.

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

Helped-by Junio C Hamano <[email protected]>

---
 ref-filter.c | 10 ++++++++++
 1 file changed, 10 insertions(+)

ref-filter.c +10 -0

index ece16a1..91a6ec9 100644
@@ -904,6 +904,16 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
+{
+	return for_each_ref_fn(ref_filter_handler, data);
+}
+
 static int cmp_ref_sorting(struct ref_sorting *s, struct ref_array_item *a, struct ref_array_item *b)
 {
 	struct atom_value *va, *vb;

Signature · 1 line

2.4.2

Karthik Nayak

author

2016-06-15 23:05 UTC #15 lore raw

PATCH v6 10/11 for-each-ref: introduce filter_refs()

Introduce filter_refs() which will act as an API for users to call and provide a function which will iterate over a set of refs while filtering out the required refs.

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

Helped-by Junio C Hamano <[email protected]>

---
 ref-filter.c | 12 +++++++++++-
 ref-filter.h |  8 ++++++--
 2 files changed, 17 insertions(+), 3 deletions(-)

ref-filter.c +11 -1

index ece16a1..886c3d7 100644
@@ -858,7 +858,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.
  */
-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;

@@ -904,6 +904,16 @@ void ref_array_clear(struct ref_array *array)
 	array->nr = array->alloc = 0;
 }
 
+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
+{
+	return for_each_ref_fn(ref_filter_handler, data);
+}
+
 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 +6 -2

index c2c5d37..15e6766 100644
@@ -46,8 +46,12 @@ struct ref_filter_cbdata {
 	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. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data);
 /*  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

Junio C Hamano

2016-06-15 23:05 UTC #16 lore raw

Re: [PATCH v6 10/11] for-each-ref: introduce filter_refs()

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

Quoted 9 lines · Karthik Nayak

ref-filter.c +9 -0

+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
+{
+	return for_each_ref_fn(ref_filter_handler, data);
+}

I do not think it is such a good idea to allow API callers to specify for-each-ref-fn directly. See my message in an earlier review.

I also think ref_filter_cbdata is an implementation detail of filter_refs and may not have to be exposed to the API callers. It probably is more sensible for them to pass

- an array of refs to receive filtered results (your ref_array thing)
- the criteria to use when filtering (your ref_filter thing)

as two separate parameters to this function, together with other parameters that lets you (meaning the implementation of filter_refs()) to decide which for-each-ref iterator to call, e.g. do you want to use raw iteration? do you want to iterate only over refs/heads? etc.

In other words, the caller of this API should not have to know that you (meaning the implementation of filter_refs()) are internally using for_each_ref() API.

Karthik Nayak

author

2016-06-15 23:05 UTC #17 lore raw

Re: [PATCH v6 10/11] for-each-ref: introduce filter_refs()

Quoted 16 lines · Junio C Hamano

ref-filter.c void ref_array_clear(struct ref_array *array) · line 915
Quoted 9 lines · Karthik Nayak
ref-filter.c +9 -0
+/*
+ * API for filtering a set of refs. The refs are provided and iterated
+ * over using the for_each_ref_fn(). The refs are stored into and filtered
+ * based on the ref_filter_cbdata structure.
+ */
+int filter_refs(int (for_each_ref_fn)(each_ref_fn, void *), struct ref_filter_cbdata *data)
+{
+	return for_each_ref_fn(ref_filter_handler, data);
+}
I do not think it is such a good idea to allow API callers to specify for-each-ref-fn directly. See my message in an earlier review.

I did read your previous message. I misunderstood some things.

Quoted 8 lines

I also think ref_filter_cbdata is an implementation detail of filter_refs and may not have to be exposed to the API callers. It probably is more sensible for them to pass
- an array of refs to receive filtered results (your ref_array thing)
- the criteria to use when filtering (your ref_filter thing)

This could be done.

Quoted 10 lines

as two separate parameters to this function, together with other parameters that lets you (meaning the implementation of filter_refs()) to decide which for-each-ref iterator to call, e.g. do you want to use raw iteration? do you want to iterate only over refs/heads? etc.
In other words, the caller of this API should not have to know that you (meaning the implementation of filter_refs()) are internally using for_each_ref() API.

I'm a little confused about this, how exactly do you propose we go about doing something like this? I mean, usually the user of the API
knows what exactly they want, like in tag.c, branch.c and for-each-ref.c
But I'm not sure what you mean by "parameters that lets you (meaning the implementation of filter_refs()) to decide which for-each-ref iterator to call". A small example maybe? Thanks!

Signature · 2 lines

Regards,
Karthik

Karthik Nayak

author

2016-06-15 23:05 UTC #18 lore raw

PATCH v6 11/11 ref-filter: make 'ref_array_item' use a FLEX_ARRAY for refname

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 91a6ec9..745d3b3 100644
@@ -846,8 +846,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;

@@ -888,7 +890,6 @@ int ref_filter_handler(const char *refname, const struct object_id *oid, int fla
 static void free_array_item(struct ref_array_item *item)
 {
 	free(item->symref);
-	free(item->refname);
 	free(item);
 }

ref-filter.h +1 -1

index 15e6766..041a39a 100644
@@ -29,7 +29,7 @@ struct ref_array_item {
 	int flag;
 	char *symref;
 	struct atom_value *value;
-	char *refname;
+	char refname[FLEX_ARRAY];
 };
 
 struct ref_array {

Signature · 1 line

2.4.2

Junio C Hamano

2016-06-15 23:05 UTC #19 lore raw

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

Quoted 1 line · Karthik Nayak

+ * Given a refname, return 1 if the refname matches with one of the patterns

You can "match refname with pattern". But "refname matches the pattern" without "with", I think.

I am not a native, though.

Quoted 2 lines

+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.

s/wild/wildcard/.

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).

perhaps?

Karthik Nayak

author

2016-06-15 23:05 UTC #20 lore raw

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

Quoted 14 lines · Junio C Hamano

Quoted 1 line · Karthik Nayak
+ * Given a refname, return 1 if the refname matches with one of the patterns
You can "match refname with pattern". But "refname matches the pattern" without "with", I think.
I am not a native, though.
Quoted 2 lines
+ * while the pattern is a pathname like 'refs/tags' or 'refs/heads/master'
+ * and so on, else return 0. Supports use of wild characters.
s/wild/wildcard/.

Quoted 8 lines

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).
perhaps?

Sounds a lot better, thanks :)

Signature · 2 lines

Regards,
Karthik

Karthik Nayak

author

2016-06-15 23:05 UTC #21 lore raw

Quoted 6 lines · Karthik Nayak

Version four of this patch can be found here :
http://www.mail-archive.com/[email protected]/msg70280.html
v5 : http://www.mail-archive.com/[email protected]/msg70888.html
Changes in v5:

Quoted 8 lines

*    Rename functions to better suit the code.
*    implement filter_refs()
*    use FLEX_ARRAY for refname
*    other small changes
Changes in v6:
* based on latest master branch (github.com/git/git)
* rename variables named sort to sorting.

Patch 09/11 and 10/11 had a some code swapped, weird. Have mailed new patches.

Signature · 2 lines

Regards,
Karthik

Matthieu Moy

2016-06-15 23:05 UTC #22 lore raw