pack-write: refactor renaming in finish_tmp_packfile()

Refactor the renaming in finish_tmp_packfile() into a helper function.
The callers are now expected to pass a "name_buffer" ending in
"pack-OID." instead of the previous "pack-", we then append "pack",
"idx" or "rev" to it.

By doing the strbuf_setlen() in rename_tmp_packfile() we reuse the
buffer and avoid the repeated allocations we'd get if that function had
its own temporary "struct strbuf".

This approach of reusing the buffer does make the last user in
pack-object.c's write_pack_file() slightly awkward, since we needlessly
do a strbuf_setlen() before calling strbuf_release() for consistency. In
subsequent changes we'll move that bitmap writing code around, so let's
not skip the strbuf_setlen() now.

The previous strbuf_reset() idiom originated with 5889271114a
(finish_tmp_packfile():use strbuf for pathname construction,
2014-03-03), which in turn was a minimal adjustment of pre-strbuf code
added in 0e990530ae (finish_tmp_packfile(): a helper function,
2011-10-28).

Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index de00adbb9e0d517bf44d6318ec8df26b9f82c503..d42de63fda407ef4714dee8bc7eeea0e7a857ddf 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -1237,7 +1237,8 @@ static void write_pack_file(void)
                                        warning_errno(_("failed utime() on %s"), pack_tmp_name);
                        }
 
-                       strbuf_addf(&tmpname, "%s-", base_name);
+                       strbuf_addf(&tmpname, "%s-%s.", base_name,
+                                   hash_to_hex(hash));
 
                        if (write_bitmap_index) {
                                bitmap_writer_set_checksum(hash);
@@ -1250,8 +1251,9 @@ static void write_pack_file(void)
                                            &pack_idx_opts, hash);
 
                        if (write_bitmap_index) {
-                               strbuf_addf(&tmpname, "%s.bitmap", hash_to_hex(hash));
+                               size_t tmpname_len = tmpname.len;
 
+                               strbuf_addstr(&tmpname, "bitmap");
                                stop_progress(&progress_state);
 
                                bitmap_writer_show_progress(progress);
@@ -1260,6 +1262,7 @@ static void write_pack_file(void)
                                bitmap_writer_finish(written_list, nr_written,
                                                     tmpname.buf, write_bitmap_options);
                                write_bitmap_index = 0;
+                               strbuf_setlen(&tmpname, tmpname_len);
                        }
 
                        strbuf_release(&tmpname);

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 6283bc8bd967d444424ddc63c5b61f6139d3b0dc..c19d471f0bde9839466456fb375ad4ca0c88d0c2 100644 (file)
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -46,7 +46,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
                close(fd);
        }
 
-       strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());
+       strbuf_addf(&packname, "%s/pack/pack-%s.", get_object_directory(),
+                   hash_to_hex(hash));
        finish_tmp_packfile(&packname, state->pack_tmp_name,
                            state->written, state->nr_written,
                            &state->pack_idx_opts, hash);

diff --git a/pack-write.c b/pack-write.c
index f1fc3ecafadef49d73e6efe7451c683d6e3ccd39..ca94e2b106cde8eecc6c8ac80d8e70fbbdc5c7ed 100644 (file)
--- a/pack-write.c
+++ b/pack-write.c
@@ -462,6 +462,18 @@ struct hashfile *create_tmp_packfile(char **pack_tmp_name)
        return hashfd(fd, *pack_tmp_name);
 }
 
+static void rename_tmp_packfile(struct strbuf *name_prefix, const char *source,
+                               const char *ext)
+{
+       size_t name_prefix_len = name_prefix->len;
+
+       strbuf_addstr(name_prefix, ext);
+       if (rename(source, name_prefix->buf))
+               die_errno("unable to rename temporary file to '%s'",
+                         name_prefix->buf);
+       strbuf_setlen(name_prefix, name_prefix_len);
+}
+
 void finish_tmp_packfile(struct strbuf *name_buffer,
                         const char *pack_tmp_name,
                         struct pack_idx_entry **written_list,
@@ -470,7 +482,6 @@ void finish_tmp_packfile(struct strbuf *name_buffer,
                         unsigned char hash[])
 {
        const char *idx_tmp_name, *rev_tmp_name = NULL;
-       int basename_len = name_buffer->len;
 
        if (adjust_shared_perm(pack_tmp_name))
                die_errno("unable to make temporary pack file readable");
@@ -483,26 +494,10 @@ void finish_tmp_packfile(struct strbuf *name_buffer,
        rev_tmp_name = write_rev_file(NULL, written_list, nr_written, hash,
                                      pack_idx_opts->flags);
 
-       strbuf_addf(name_buffer, "%s.pack", hash_to_hex(hash));
-
-       if (rename(pack_tmp_name, name_buffer->buf))
-               die_errno("unable to rename temporary pack file");
-
-       strbuf_setlen(name_buffer, basename_len);
-
-       strbuf_addf(name_buffer, "%s.idx", hash_to_hex(hash));
-       if (rename(idx_tmp_name, name_buffer->buf))
-               die_errno("unable to rename temporary index file");
-
-       strbuf_setlen(name_buffer, basename_len);
-
-       if (rev_tmp_name) {
-               strbuf_addf(name_buffer, "%s.rev", hash_to_hex(hash));
-               if (rename(rev_tmp_name, name_buffer->buf))
-                       die_errno("unable to rename temporary reverse-index file");
-       }
-
-       strbuf_setlen(name_buffer, basename_len);
+       rename_tmp_packfile(name_buffer, pack_tmp_name, "pack");
+       rename_tmp_packfile(name_buffer, idx_tmp_name, "idx");
+       if (rev_tmp_name)
+               rename_tmp_packfile(name_buffer, rev_tmp_name, "rev");
 
        free((void *)idx_tmp_name);
 }