Re: [PATCH, v4] git-tag: introduce --cleanup option

[Junio C Hamano <gitster@pobox.com>]

Jeff King <peff@peff.net> writes:

> More importantly, though, this seems to break t6300 badly. I haven't
> looked into why yet, though.

Probably two issues.

 - opt.message (and the original 'message') was misnamed and confused the
   patch author what "if (!message && !buf->len)" meant.

 - "opt" is a structure meant to be extensible, but is not initialized as
   a whole, inviting future errors.

It still seems to be broken with respect to the primary thing the patch
wanted to do (t7400 "git tag -F commentsfile comments-annotated-tag" does
not seem to produce an expected result), so I'll kick it back to the
Kirill to look at.

Thanks.

 builtin/tag.c |   12 ++++++------
 1 files changed, 6 insertions(+), 6 deletions(-)

diff --git a/builtin/tag.c b/builtin/tag.c
index 27a66a3..7883720 100644
--- a/builtin/tag.c
+++ b/builtin/tag.c
@@ -329,7 +329,7 @@ static int build_tag_object(struct strbuf *buf, int sign, unsigned char *result)
 }
 
 struct create_tag_options {
-	unsigned int message;
+	unsigned int message_given:1;
 	unsigned int sign;
 	enum {
 		CLEANUP_NONE,
@@ -364,7 +364,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 	if (header_len > sizeof(header_buf) - 1)
 		die(_("tag header too big."));
 
-	if (!opt->message) {
+	if (!opt->message_given) {
 		int fd;
 
 		/* write the template message before editing: */
@@ -393,7 +393,7 @@ static void create_tag(const unsigned char *object, const char *tag,
 	if (opt->cleanup_mode != CLEANUP_NONE)
 		stripspace(buf, opt->cleanup_mode == CLEANUP_ALL);
 
-	if (opt->message && !buf->len)
+	if (!opt->message_given && !buf->len)
 		die(_("no tag message?"));
 
 	strbuf_insert(buf, 0, header_buf, header_len);
@@ -486,7 +486,7 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 
 	git_config(git_tag_config, NULL);
 
-	opt.sign = 0;
+	memset(&opt, 0, sizeof(opt));
 
 	argc = parse_options(argc, argv, prefix, options, git_tag_usage, 0);
 
@@ -552,10 +552,10 @@ int cmd_tag(int argc, const char **argv, const char *prefix)
 	else if (!force)
 		die(_("tag '%s' already exists"), tag);
 
-	opt.message = msg.given || msgfile;
+	opt.message_given = msg.given || msgfile;
 
 	if (!cleanup_arg || !strcmp(cleanup_arg, "default"))
-		opt.cleanup_mode = !opt.message ? CLEANUP_ALL : CLEANUP_SPACE;
+		opt.cleanup_mode = !opt.message_given ? CLEANUP_ALL : CLEANUP_SPACE;
 	else if (!strcmp(cleanup_arg, "verbatim"))
 		opt.cleanup_mode = CLEANUP_NONE;
 	else if (!strcmp(cleanup_arg, "whitespace"))