git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH 01/15] check return value of verify_ref_format()

From
Jeff King <peff@peff.net>
Date
Jul 13, 2017, 14:56 UTC
Message-ID
<20170713145610.gpomty7guf54lmsb@sigill.intra.peff.net>
In-Reply-To
<20170713145553.3epnsw23zajwg3ee@sigill.intra.peff.net>

Users of the ref-filter code must call verify_ref_format() before formatting any refs, but most ignore its return value. This means we may print an error on a syntactically bogus pattern, but keep going anyway.

In most cases this results in a fatal error when we actually try to format a ref. But if you have no refs to show at all, then the behavior is confusing: git prints the error from verify_ref_format(), then exits with code 0 without showing any output. Let's instead abort immediately if we know we have a bogus format.

We'll output the usage information if we have it handy (just like the existing call in cmd_for_each_ref() does), and otherwise just die().

Signed-off-by: Jeff King <peff@peff.net>
---
 builtin/branch.c     | 4 +++-
 builtin/tag.c        | 7 ++++---
 builtin/verify-tag.c | 4 +++-
 3 files changed, 10 insertions(+), 5 deletions(-)
diff --git a/builtin/branch.c b/builtin/branch.c
index 8a0595e11..e756a5667 100644
--- a/builtin/branch.c
+++ b/builtin/branch.c
@@ -409,7 +409,9 @@ static void print_ref_list(struct ref_filter *filter, struct ref_sorting *sortin
 
 	if (!format)
 		format = to_free = build_format(filter, maxwidth, remote_prefix);
-	verify_ref_format(format);
+
+	if (verify_ref_format(format))
+		die(_("unable to parse format string"));
 
 	ref_array_sort(sorting, &array);
 
diff --git a/builtin/tag.c b/builtin/tag.c
index 01154ea8d..216629fb2 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -53,7 +53,8 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 			format = "%(refname:lstrip=2)";
 	}
 
-	verify_ref_format(format);
+	if (verify_ref_format(format))
+		die(_("unable to parse format string"));
 	filter->with_commit_tag_algo = 1;
 	filter_refs(&array, filter, FILTER_REFS_TAGS);
 	ref_array_sort(sorting, &array);
@@ -501,8 +502,8 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (cmdmode == 'd')
 		return for_each_tag_name(argv, delete_tag, NULL);
 	if (cmdmode == 'v') {
-		if (format)
-			verify_ref_format(format);
+		if (format && verify_ref_format(format))
+			usage_with_options(git_tag_usage, options);
 		return for_each_tag_name(argv, verify_tag, format);
 	}
 
diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index f9a5f7535..a10eca2b2 100644
--- a/builtin/verify-tag.c
+++ b/builtin/verify-tag.c
@@ -51,7 +51,9 @@ int cmd_verify_tag(int argc, const char **argv, const char *prefix)
 		flags |= GPG_VERIFY_VERBOSE;
 
 	if (fmt_pretty) {
-		verify_ref_format(fmt_pretty);
+		if (verify_ref_format(fmt_pretty))
+			usage_with_options(verify_tag_usage,
+					   verify_tag_options);
 		flags |= GPG_VERIFY_OMIT_STATUS;
 	}
 
-- 
2.13.2.1157.gc6daca446
Previous: Jeff KingNext: Jeff King
Message 2 of 30 in “making user-format colors conditional on config/tty”
  1. 0/15 making user-format colors conditional on config/ttyJeff King, Jul 13, 2017
  2. 01/15 check return value of verify_ref_format()Jeff King, Jul 13, 2017
  3. 02/15 docs/for-each-ref: update pointer to color syntaxJeff King, Jul 13, 2017
  4. Junio C HamanoJul 13, 2017
  5. 03/15 t: use test_decode_color rather than literal ANSI codesJeff King, Jul 13, 2017
  6. Stefan BellerJul 13, 2017
  7. Jeff KingJul 13, 2017
  8. Junio C HamanoJul 13, 2017
  9. Jeff KingJul 14, 2017
  10. Junio C HamanoJul 13, 2017
  11. 04/15 ref-filter: simplify automatic color resetJeff King, Jul 13, 2017
  12. 05/15 ref-filter: abstract ref format into its own structJeff King, Jul 13, 2017
  13. Stefan BellerJul 13, 2017
  14. Junio C HamanoJul 13, 2017
  15. Junio C HamanoJul 13, 2017
  16. Stefan BellerJul 13, 2017
  17. Junio C HamanoJul 13, 2017
  18. 06/15 ref-filter: move need_color_reset_at_eol into ref_formatJeff King, Jul 13, 2017
  19. Junio C HamanoJul 13, 2017
  20. 07/15 ref-filter: provide a function for parsing sort optionsJeff King, Jul 13, 2017
  21. 08/15 ref-filter: make parse_ref_filter_atom a private functionJeff King, Jul 13, 2017
  22. 09/15 ref-filter: factor out the parsing of sorting atomsJeff King, Jul 13, 2017
  23. 10/15 ref-filter: pass ref_format struct to atom parsersJeff King, Jul 13, 2017
  24. 11/15 color: check color.ui in git_default_config()Jeff King, Jul 13, 2017
  25. 12/15 for-each-ref: load config earlierJeff King, Jul 13, 2017
  26. 13/15 rev-list: pass diffopt->use_colors through to pretty-printJeff King, Jul 13, 2017
  27. 14/15 pretty: respect color settings for %C placeholdersJeff King, Jul 13, 2017
  28. 15/15 ref-filter: consult want_color() before emitting colorsJeff King, Jul 13, 2017
  29. Stefan BellerJul 13, 2017
  30. Junio C HamanoJul 13, 2017

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.