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

> 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);
>  }