threads / patch / 18250

patchbuiltin-tag.c: remove global variable to use the callback data of git-config.

Subject: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.

## tl;dr

4 messages between Mar 10, 2009 and Mar 10, 2009. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Carlos Rica· Mar 10, 2009, 13:03 UTC · lore
Signed-off-by: Carlos Rica <jasampler@yahoo.es>
---
This way the data flow is much clearer.

Here I declare a struct to wrap the new local array along with its size. An alternative to this is strbuf, would it be preferable?

 builtin-tag.c |   43 ++++++++++++++++++++++++++-----------------
 1 files changed, 26 insertions(+), 17 deletions(-)
Show changes to builtin-tag.c +26 −17
diff --git a/builtin-tag.c b/builtin-tag.c
index 01e7374..2b2d728 100644
--- a/builtin-tag.c
+++ b/builtin-tag.c
@@ -21,8 +21,6 @@ static const char * const git_tag_usage[] = {
 	NULL
 };
 
-static char signingkey[1000];
-
 struct tag_filter {
 	const char *pattern;
 	int lines;
@@ -156,7 +154,12 @@ static int verify_tag(const char *name, const char
*ref,
 	return 0;
 }
 
-static int do_sign(struct strbuf *buffer)
+struct char_array {
+	char *buf;
+	size_t size;
+};
+
+static int do_sign(struct char_array *signingkey, struct strbuf
*buffer)
 {
 	struct child_process gpg;
 	const char *args[4];
@@ -164,11 +167,12 @@ static int do_sign(struct strbuf *buffer)
 	int len;
 	int i, j;
 
-	if (!*signingkey) {
-		if (strlcpy(signingkey, git_committer_info(IDENT_ERROR_ON_NO_NAME),
-				sizeof(signingkey)) > sizeof(signingkey) - 1)
+	if (!signingkey->buf[0]) {
+		if (strlcpy(signingkey->buf,
+				git_committer_info(IDENT_ERROR_ON_NO_NAME),
+				signingkey->size) > signingkey->size - 1)
 			return error("committer info too long.");
-		bracket = strchr(signingkey, '>');
+		bracket = strchr(signingkey->buf, '>');
 		if (bracket)
 			bracket[1] = '\0';
 	}
@@ -183,7 +187,7 @@ static int do_sign(struct strbuf *buffer)
 	gpg.out = -1;
 	args[0] = "gpg";
 	args[1] = "-bsau";
-	args[2] = signingkey;
+	args[2] = signingkey->buf;
 	args[3] = NULL;
 
 	if (start_command(&gpg))
@@ -220,9 +224,10 @@ static const char tag_template[] =
 	"# Write a tag message\n"
 	"#\n";
 
-static void set_signingkey(const char *value)
+static void set_signingkey(struct char_array *signingkey, const char
*value)
 {
-	if (strlcpy(signingkey, value, sizeof(signingkey)) >=
sizeof(signingkey))
+	if (strlcpy(signingkey->buf, value, signingkey->size)
+					>= signingkey->size)
 		die("signing key value too long (%.10s...)", value);
 }
 
@@ -231,7 +236,7 @@ static int git_tag_config(const char *var, const
char *value, void *cb)
 	if (!strcmp(var, "user.signingkey")) {
 		if (!value)
 			return config_error_nonbool(var);
-		set_signingkey(value);
+		set_signingkey((struct char_array *) cb, value);
 		return 0;
 	}
 
@@ -266,9 +271,10 @@ static void write_tag_body(int fd, const unsigned
char *sha1)
 	free(buf);
 }
 
-static int build_tag_object(struct strbuf *buf, int sign, unsigned char
*result)
+static int build_tag_object(struct strbuf *buf, int sign,
+			struct char_array *signingkey, unsigned char *result)
 {
-	if (sign && do_sign(buf) < 0)
+	if (sign && do_sign(signingkey, buf) < 0)
 		return error("unable to sign the tag");
 	if (write_sha1_file(buf->buf, buf->len, tag_type, result) < 0)
 		return error("unable to write tag file");
@@ -277,6 +283,7 @@ static int build_tag_object(struct strbuf *buf, int
sign, unsigned char *result)
 
 static void create_tag(const unsigned char *object, const char *tag,
 		       struct strbuf *buf, int message, int sign,
+		       struct char_array *signingkey,
 		       unsigned char *prev, unsigned char *result)
 {
 	enum object_type type;
@@ -331,7 +338,7 @@ static void create_tag(const unsigned char *object,
const char *tag,
 
 	strbuf_insert(buf, 0, header_buf, header_len);
 
-	if (build_tag_object(buf, sign, result) < 0) {
+	if (build_tag_object(buf, sign, signingkey, result) < 0) {
 		if (path)
 			fprintf(stderr, "The tag message has been left in %s\n",
 				path);
@@ -374,6 +381,8 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
 	const char *msgfile = NULL, *keyid = NULL;
 	struct msg_arg msg = { 0, STRBUF_INIT };
 	struct commit_list *with_commit = NULL;
+	char keyarr[1000] = {'\0'};
+	struct char_array signingkey = { keyarr, sizeof(keyarr) };
 	struct option options[] = {
 		OPT_BOOLEAN('l', NULL, &list, "list tag names"),
 		{ OPTION_INTEGER, 'n', NULL, &lines, NULL,
@@ -403,14 +412,14 @@ int cmd_tag(int argc, const char **argv, const
char *prefix)
 		OPT_END()
 	};
 
-	git_config(git_tag_config, NULL);
+	git_config(git_tag_config, &signingkey);
 
 	argc = parse_options(argc, argv, options, git_tag_usage, 0);
 	msgfile = parse_options_fix_filename(prefix, msgfile);
 
 	if (keyid) {
 		sign = 1;
-		set_signingkey(keyid);
+		set_signingkey(&signingkey, keyid);
 	}
 	if (sign)
 		annotate = 1;
@@ -474,7 +483,7 @@ int cmd_tag(int argc, const char **argv, const char
*prefix)
 
 	if (annotate)
 		create_tag(object, tag, &buf, msg.given || msgfile,
-			   sign, prev, object);
+			   sign, &signingkey, prev, object);
 
 	lock = lock_any_ref_for_update(ref, prev, 0);
 	if (!lock)
-- 
1.5.4.3
Johannes Schindelin· Mar 10, 2009, 13:34 UTC · re: Carlos Rica · lore

Re: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.

Hi,
On Tue, 10 Mar 2009, Carlos Rica wrote:
> 
> Signed-off-by: Carlos Rica <jasampler@yahoo.es>
Good to see you again!

BTW do you want to be recorded with a different email address in the author line than in the S-O-B?

Show 5 quoted lines
> -static int do_sign(struct strbuf *buffer)
> +struct char_array {
> +	char *buf;
> +	size_t size;
> +};
That looks very much like you want a struct strbuf, no?

Ciao, Dscho

Carlos Rica· Mar 10, 2009, 14:00 UTC · re: Johannes Schindelin · lore

Re: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.

Please, don't even try to apply this patch, since long lines are wrapped.
I will send it fixed, sorry by the inconvenience.

On Tue, Mar 10, 2009 at 2:34 PM, Johannes Schindelin <Johannes.Schindelin@gmx.de> wrote:

Show 8 quoted lines
> Hi,
>
> On Tue, 10 Mar 2009, Carlos Rica wrote:
>
>>
>> Signed-off-by: Carlos Rica <jasampler@yahoo.es>
>
> Good to see you again!
Thanks!
>
> BTW do you want to be recorded with a different email address in the
> author line than in the S-O-B?

Thank you for notify it, Johannes! That's was another mistake here, I wanted to use the gmail account for this, I will change it when I send this fixed.

Show 7 quoted lines
>> -static int do_sign(struct strbuf *buffer)
>> +struct char_array {
>> +     char *buf;
>> +     size_t size;
>> +};
>
> That looks very much like you want a struct strbuf, no?

I was asking exactly that, in the hidden lines of the patch, so if everybody prefers the strbuf solution I will use it, and then, we should choose if there must be a limit for the signing key id max length (now 1000) since by using dynamic memory it would not be required.

> Ciao,
> Dscho
>
>
Junio C Hamano· Mar 10, 2009, 16:36 UTC · re: Carlos Rica · lore

Re: [PATCH] builtin-tag.c: remove global variable to use the callback data of git-config.

Carlos Rica <jasampler@gmail.com> writes:
> Signed-off-by: Carlos Rica <jasampler@yahoo.es>
> ---
>
> This way the data flow is much clearer.
Good.  I think the Subject is backwards, though.

← back to recent threads