finish_tmp_packfile():use strbuf for pathname construction

The old version fixes a maximum length on the buffer, which could be a problem
if one is not certain of the length of get_object_directory().
Using strbuf can avoid the protential bug.

Helped-by: Michael Haggerty <mhagger@alum.mit.edu>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Signed-off-by: Sun He <sunheehnus@gmail.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c
index 4922ce5e276a69bc5ab424dd123b63db67e7aa11..3f427b4e4ba35c2313c149903bb173c675a56e2d 100644 (file)
--- a/builtin/pack-objects.c
+++ b/builtin/pack-objects.c
@@ -803,7 +803,7 @@ static void write_pack_file(void)
 
                if (!pack_to_stdout) {
                        struct stat st;
-                       char tmpname[PATH_MAX];
+                       struct strbuf tmpname = STRBUF_INIT;
 
                        /*
                         * Packs are runtime accessed in their mtime
@@ -826,23 +826,19 @@ static void write_pack_file(void)
                                                pack_tmp_name, strerror(errno));
                        }
 
-                       /* Enough space for "-<sha-1>.pack"? */
-                       if (sizeof(tmpname) <= strlen(base_name) + 50)
-                               die("pack base name '%s' too long", base_name);
-                       snprintf(tmpname, sizeof(tmpname), "%s-", base_name);
+                       strbuf_addf(&tmpname, "%s-", base_name);
 
                        if (write_bitmap_index) {
                                bitmap_writer_set_checksum(sha1);
                                bitmap_writer_build_type_index(written_list, nr_written);
                        }
 
-                       finish_tmp_packfile(tmpname, pack_tmp_name,
+                       finish_tmp_packfile(&tmpname, pack_tmp_name,
                                            written_list, nr_written,
                                            &pack_idx_opts, sha1);
 
                        if (write_bitmap_index) {
-                               char *end_of_name_prefix = strrchr(tmpname, 0);
-                               sprintf(end_of_name_prefix, "%s.bitmap", sha1_to_hex(sha1));
+                               strbuf_addf(&tmpname, "%s.bitmap", sha1_to_hex(sha1));
 
                                stop_progress(&progress_state);
 
@@ -851,10 +847,11 @@ static void write_pack_file(void)
                                bitmap_writer_select_commits(indexed_commits, indexed_commits_nr, -1);
                                bitmap_writer_build(&to_pack);
                                bitmap_writer_finish(written_list, nr_written,
-                                                    tmpname, write_bitmap_options);
+                                                    tmpname.buf, write_bitmap_options);
                                write_bitmap_index = 0;
                        }
 
+                       strbuf_release(&tmpname);
                        free(pack_tmp_name);
                        puts(sha1_to_hex(sha1));
                }

diff --git a/bulk-checkin.c b/bulk-checkin.c
index 118c62528b0bb6c919d146d60516825a5336fe75..98e651c284254b3c96c59b4f444cbc0f54edd90e 100644 (file)
--- a/bulk-checkin.c
+++ b/bulk-checkin.c
@@ -4,6 +4,7 @@
 #include "bulk-checkin.h"
 #include "csum-file.h"
 #include "pack.h"
+#include "strbuf.h"
 
 static int pack_compression_level = Z_DEFAULT_COMPRESSION;
 
@@ -23,7 +24,7 @@ static struct bulk_checkin_state {
 static void finish_bulk_checkin(struct bulk_checkin_state *state)
 {
        unsigned char sha1[20];
-       char packname[PATH_MAX];
+       struct strbuf packname = STRBUF_INIT;
        int i;
 
        if (!state->f)
@@ -43,8 +44,8 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
                close(fd);
        }
 
-       sprintf(packname, "%s/pack/pack-", get_object_directory());
-       finish_tmp_packfile(packname, state->pack_tmp_name,
+       strbuf_addf(&packname, "%s/pack/pack-", get_object_directory());
+       finish_tmp_packfile(&packname, state->pack_tmp_name,
                            state->written, state->nr_written,
                            &state->pack_idx_opts, sha1);
        for (i = 0; i < state->nr_written; i++)
@@ -54,6 +55,7 @@ static void finish_bulk_checkin(struct bulk_checkin_state *state)
        free(state->written);
        memset(state, 0, sizeof(*state));
 
+       strbuf_release(&packname);
        /* Make objects we just wrote available to ourselves */
        reprepare_packed_git();
 }

diff --git a/pack-write.c b/pack-write.c
index 9b8308b7594263c4900a6d75c18b91cea959fd70..9ccf80419b06789aa3a1171869bd4179787e5eff 100644 (file)
--- a/pack-write.c
+++ b/pack-write.c
@@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char **pack_tmp_name)
        return sha1fd(fd, *pack_tmp_name);
 }
 
-void finish_tmp_packfile(char *name_buffer,
+void finish_tmp_packfile(struct strbuf *name_buffer,
                         const char *pack_tmp_name,
                         struct pack_idx_entry **written_list,
                         uint32_t nr_written,
@@ -344,7 +344,7 @@ void finish_tmp_packfile(char *name_buffer,
                         unsigned char sha1[])
 {
        const char *idx_tmp_name;
-       char *end_of_name_prefix = strrchr(name_buffer, 0);
+       int basename_len = name_buffer->len;
 
        if (adjust_shared_perm(pack_tmp_name))
                die_errno("unable to make temporary pack file readable");
@@ -354,17 +354,19 @@ void finish_tmp_packfile(char *name_buffer,
        if (adjust_shared_perm(idx_tmp_name))
                die_errno("unable to make temporary index file readable");
 
-       sprintf(end_of_name_prefix, "%s.pack", sha1_to_hex(sha1));
-       free_pack_by_name(name_buffer);
+       strbuf_addf(name_buffer, "%s.pack", sha1_to_hex(sha1));
+       free_pack_by_name(name_buffer->buf);
 
-       if (rename(pack_tmp_name, name_buffer))
+       if (rename(pack_tmp_name, name_buffer->buf))
                die_errno("unable to rename temporary pack file");
 
-       sprintf(end_of_name_prefix, "%s.idx", sha1_to_hex(sha1));
-       if (rename(idx_tmp_name, name_buffer))
+       strbuf_setlen(name_buffer, basename_len);
+
+       strbuf_addf(name_buffer, "%s.idx", sha1_to_hex(sha1));
+       if (rename(idx_tmp_name, name_buffer->buf))
                die_errno("unable to rename temporary index file");
 
-       *end_of_name_prefix = '\0';
+       strbuf_setlen(name_buffer, basename_len);
 
        free((void *)idx_tmp_name);
 }

diff --git a/pack.h b/pack.h
index 12d951659c16462cf2da6a43e2c4974b85e39712..3223f5a0380f735509424bbdbad64097948ef85b 100644 (file)
--- a/pack.h
+++ b/pack.h
@@ -91,6 +91,6 @@ extern int encode_in_pack_object_header(enum object_type, uintmax_t, unsigned ch
 extern int read_pack_header(int fd, struct pack_header *);
 
 extern struct sha1file *create_tmp_packfile(char **pack_tmp_name);
-extern void finish_tmp_packfile(char *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
+extern void finish_tmp_packfile(struct strbuf *name_buffer, const char *pack_tmp_name, struct pack_idx_entry **written_list, uint32_t nr_written, struct pack_idx_option *pack_idx_opts, unsigned char sha1[]);
 
 #endif