Re: [PATCH v6 4/6] builtin/tag: add --format argument for tag -v

Junio C Hamano [off-list ref] writes:

santiago@nyu.edu writes:

quoted

@@ -428,9 +443,12 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	if (filter.merge_commit)
 		die(_("--merged and --no-merged option are only allowed with -l"));
 	if (cmdmode == 'd')
-		return for_each_tag_name(argv, delete_tag);
-	if (cmdmode == 'v')
-		return for_each_tag_name(argv, verify_tag);
+		return for_each_tag_name(argv, delete_tag, NULL);
+	if (cmdmode == 'v') {
+		if (format)
+			verify_ref_format(format);
+		return for_each_tag_name(argv, verify_tag, format);
+	}

This triggers:

    builtin/tag.c: In function 'cmd_tag':
    builtin/tag.c:451:3: error: passing argument 3 of
    'for_each_tag_name' discards 'const' qualifier from pointer target type [-Werror]
       return for_each_tag_name(argv, verify_tag, format);

Either for-each-tag-name's new parameter needs to be typed
correctly, or the type of the "format" variable needs to be updated.

Squashing the following into this commit solves this issue with the
former approach.  The lines it touches are all from 4/6 and I view
all of it as general improvement, including type correctness and
code formatting.

diff --git a/builtin/tag.c b/builtin/tag.c
index f81273a85a..fbb85ba3dc 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c

@@ -66,10 +66,10 @@ static int list_tags(struct ref_filter *filter, struct ref_sorting *sorting, con
 }
 
 typedef int (*each_tag_name_fn)(const char *name, const char *ref,
-				const unsigned char *sha1, void *cb_data);
+				const unsigned char *sha1, const void *cb_data);
 
 static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
-		void *cb_data)
+			     const void *cb_data)
 {
 	const char **p;
 	char ref[PATH_MAX];

@@ -95,7 +95,7 @@ static int for_each_tag_name(const char **argv, each_tag_name_fn fn,
 }
 
 static int delete_tag(const char *name, const char *ref,
-				const unsigned char *sha1, void *cb_data)
+		      const unsigned char *sha1, const void *cb_data)
 {
 	if (delete_ref(ref, sha1, 0))
 		return 1;

@@ -104,10 +104,10 @@ static int delete_tag(const char *name, const char *ref,
 }
 
 static int verify_tag(const char *name, const char *ref,
-				const unsigned char *sha1, void *cb_data)
+		      const unsigned char *sha1, const void *cb_data)
 {
 	int flags;
-	char *fmt_pretty = cb_data;
+	const char *fmt_pretty = cb_data;
 	flags = GPG_VERIFY_VERBOSE;
 
 	if (fmt_pretty)

On Wed, Jan 18, 2017 at 10:25:59AM -0800, Junio C Hamano wrote:

This would bite us later if one of the iterators really does need to
pass something mutable. But as this iteration interface is confined to
builtin/tag.c, I think it's a nice simple fix.

A more general fix would be to pass a non-const pointer to const pointer
(preferably inside a struct for readability). But I don't see any need
for that complexity here.

-Peff

(no body)