Re: [PATCH v11 25/41] fast-import.c: use a ref transaction when dumping tags

[Jonathan Nieder <jrnieder@gmail.com>]

Ronnie Sahlberg wrote:

> --- a/fast-import.c
> +++ b/fast-import.c
> @@ -1735,15 +1735,22 @@ static void dump_tags(void)
>  {
>  	static const char *msg = "fast-import";
>  	struct tag *t;
> -	struct ref_lock *lock;
>  	char ref_name[PATH_MAX];
> +	struct strbuf err = STRBUF_INIT;
> +	struct ref_transaction *transaction;
>  
> +	transaction = ref_transaction_begin(&err);
>  	for (t = first_tag; t; t = t->next_tag) {
> -		sprintf(ref_name, "tags/%s", t->name);
> +		snprintf(ref_name, PATH_MAX, "refs/tags/%s", t->name);

That ignores the error if the refname happens to go longer than
PATH_MAX.

This is the part of fast-import that doesn't need to be super
fast ;-).  (The objects have been written to the pack, and now
we just need to write some refs.)  Could this use a strbuf?  By that,
I mean something like

diff --git i/fast-import.c w/fast-import.c
index 3db5b3d..d5f6e63 100644
--- i/fast-import.c
+++ w/fast-import.c
@@ -1735,21 +1735,28 @@ static void dump_tags(void)
 {
 	static const char *msg = "fast-import";
 	struct tag *t;
-	char ref_name[PATH_MAX];
+	struct strbuf ref_name = STRBUF_INIT;
 	struct strbuf err = STRBUF_INIT;
 	struct ref_transaction *transaction;
 
+	strbuf_addstr(&ref_name, "refs/tags/");
+
 	transaction = ref_transaction_begin(&err);
 	for (t = first_tag; t; t = t->next_tag) {
-		snprintf(ref_name, PATH_MAX, "refs/tags/%s", t->name);
+		strbuf_setlen(&ref_name, strlen("refs/tags/"));
+		strbuf_addstr(&ref_name, t->name);
 
-		if (ref_transaction_update(transaction, ref_name, t->sha1,
-					   NULL, 0, 0, &err))
-			break;
+		if (ref_transaction_update(transaction, ref_name.buf, t->sha1,
+					   NULL, 0, 0, &err)) {
+			failure |= error("%s", err.buf);
+			goto done;
+		}
 	}
 	if (ref_transaction_commit(transaction, msg, &err))
 		failure |= error("%s", err.buf);
+done:
 	ref_transaction_free(transaction);
+	strbuf_release(&ref_name);
 	strbuf_release(&err);
 }