[PATCH] for-each-ref's new per-atom formatting was failing if there were multiple fields per line

[PATCH] for-each-ref's new per-atom formatting was failing if there were multiple fields per line

[Andy Parkins]

builtin-for-each-ref.c was searching backwards for ":" in the atom searches
performed by parse_atom.  In implementing the format handler I had
assumed that parse_atom was being handed a single atom pointer.  That
was not the case.  In fact the "atom" pointer is just a pointer within
the longer format string, that means that the NUL for the end of the
string is not at the end of the current atom, but at the end of the
format string.

Finding the ":" separating the atom name from the format was done with
strrchr(), which searches from the end of the string.  That would be
fine if it was searching a single atom string, but in the case of:

 --format="%(atom:format) %(atom:format)"

When the first atom is being parsed a backwards search actually finds
the second colon, making the atom name look like:

 "atom:format) %(atom"

Which obviously doesn't match any valid atom name and so exits with
"unknown field name".

The fix is to abandon the reverse search (which was only to allow colons
in atom names, which was redundant as there are no atom names with
colons) and replace it with a forward search.  The potential presence of
a second ":" also requires a check to confirm that the found ":" is
between the start and end pointers, which this patch also adds.

Signed-off-by: Andy Parkins <andyparkins@gmail.com>
---
 builtin-for-each-ref.c |    6 +++---
 1 files changed, 3 insertions(+), 3 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 2ca4fc6..f06d006 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -109,8 +109,8 @@ static int parse_atom(const char *atom, const char *ep)
 		/* If the atom name has a colon, strip it and everything after
 		 * it off - it specifies the format for this entry, and
 		 * shouldn't be used for checking against the valid_atom table */
-		const char *formatp = strrchr(sp, ':' );
-		if (formatp == NULL )
+		const char *formatp = strchr(sp, ':' );
+		if (formatp == NULL || formatp > ep )
 			formatp = ep;
 		if (len == formatp - sp && !memcmp(valid_atom[i].name, sp, len))
 			break;
@@ -366,7 +366,7 @@ static void grab_date(const char *buf, struct atom_value *v, const char *atomnam
 	 * it's not possible that <something> is not ":<format>" because
 	 * parse_atom() wouldn't have allowed it, so we can assume that no
 	 * ":" means no format is specified, use the default */
-	formatp = strrchr( atomname, ':' );
+	formatp = strchr( atomname, ':' );
 	if (formatp != NULL) {
 		formatp++;
 		date_mode = parse_date_format(formatp);
-- 
1.5.3.2.105.gf47f2-dirty

[PATCH] for-each-ref: fix %(numparent) and %(parent)

[Junio C Hamano]

The string value of %(numparent) was not returned correctly.
Also %(parent) misbehaved for the root commits (returned garbage)
and merge commits (returned first parent, followed by a space).

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * I noticed this while playing with Andy's patch to enhance the
   date format string we saw recently on the list.

   Andy does not have anything to do with the breakage; this
   patch is against 'maint' to fix the bug that has always been
   there from the very beginning of this code.

 builtin-for-each-ref.c |   10 ++++++----
 1 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/builtin-for-each-ref.c b/builtin-for-each-ref.c
index 0afa1c5..29f70aa 100644
--- a/builtin-for-each-ref.c
+++ b/builtin-for-each-ref.c
@@ -43,7 +43,7 @@ static struct {
 	{ "objectsize", FIELD_ULONG },
 	{ "objectname" },
 	{ "tree" },
-	{ "parent" }, /* NEEDSWORK: how to address 2nd and later parents? */
+	{ "parent" },
 	{ "numparent", FIELD_ULONG },
 	{ "object" },
 	{ "type" },
@@ -262,24 +262,26 @@ static void grab_commit_values(struct atom_value *val, int deref, struct object
 		}
 		if (!strcmp(name, "numparent")) {
 			char *s = xmalloc(40);
+			v->ul = num_parents(commit);
 			sprintf(s, "%lu", v->ul);
 			v->s = s;
-			v->ul = num_parents(commit);
 		}
 		else if (!strcmp(name, "parent")) {
 			int num = num_parents(commit);
 			int i;
 			struct commit_list *parents;
-			char *s = xmalloc(42 * num);
+			char *s = xmalloc(41 * num + 1);
 			v->s = s;
 			for (i = 0, parents = commit->parents;
 			     parents;
-			     parents = parents->next, i = i + 42) {
+			     parents = parents->next, i = i + 41) {
 				struct commit *parent = parents->item;
 				strcpy(s+i, sha1_to_hex(parent->object.sha1));
 				if (parents->next)
 					s[i+40] = ' ';
 			}
+			if (!i)
+				*s = '\0';
 		}
 	}
 }
-- 
1.5.3.3.1144.gf10f2