check return value of verify_ref_format()

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>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/branch.c b/builtin/branch.c
index 8a0595e11587aeac0f784400c43609404da3e045..e756a5667a55024f617f68409104b2cac3f978f2 100644 (file)
--- 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);
 
        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);
 
 
        ref_array_sort(sorting, &array);
 

diff --git a/builtin/tag.c b/builtin/tag.c
index 01154ea8dcca869ed635eb433d5afe879b8e883c..216629fb2ce0a3b8e2ef457c9f3b823ab12a4b0f 100644 (file)
--- 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)";
        }
 
                        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);
        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 (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);
        }
 
                return for_each_tag_name(argv, verify_tag, format);
        }
 

diff --git a/builtin/verify-tag.c b/builtin/verify-tag.c
index f9a5f7535aad9c24041e0677dfa4ed4af87cabf0..a10eca2b2d329aa223eecaa17e6edfeb80197dbc 100644 (file)
--- 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) {
                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;
        }
 
                flags |= GPG_VERIFY_OMIT_STATUS;
        }