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@xxxxxxxxx>
---
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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
