Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
2014-03-03 15:41 GMT+08:00 Eric Sunshine sunsh...@sunshineco.com: On Sat, Mar 1, 2014 at 9:29 PM, Sun He sunheeh...@gmail.com wrote: Signed-off-by: Sun He sunheeh...@gmail.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Michael Haggerty mhag...@alum.mit.edu --- This patch has assumed that you have already fix the bug of tmpname in builtin/pack-objects.c:write_pack_file() warning() builtin/pack-objects.c | 15 ++- bulk-checkin.c | 8 +--- pack-write.c | 18 ++ pack.h | 2 +- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..099d6ed 100644 --- 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) tmpname, 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)); Transpose the space and comma before the third argument. Other than this, the patch looks reasonable. OK, got it. I have already fixed it. Thank you very much 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 118c625..98e651c 100644 --- 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 @@ clear_exit: 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 9b8308b..9ccf804 100644 --- a/pack-write.c +++ b/pack-write.c @@ -336,7 +336,7 @@ struct sha1file *create_tmp_packfile(char
Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
On Sat, Mar 1, 2014 at 9:29 PM, Sun He sunheeh...@gmail.com wrote: Signed-off-by: Sun He sunheeh...@gmail.com Helped-by: Eric Sunshine sunsh...@sunshineco.com Helped-by: Michael Haggerty mhag...@alum.mit.edu --- This patch has assumed that you have already fix the bug of tmpname in builtin/pack-objects.c:write_pack_file() warning() builtin/pack-objects.c | 15 ++- bulk-checkin.c | 8 +--- pack-write.c | 18 ++ pack.h | 2 +- 4 files changed, 22 insertions(+), 21 deletions(-) diff --git a/builtin/pack-objects.c b/builtin/pack-objects.c index c733379..099d6ed 100644 --- 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) tmpname, 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)); Transpose the space and comma before the third argument. Other than this, the patch looks reasonable. 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 118c625..98e651c 100644 --- 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 @@ clear_exit: 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 9b8308b..9ccf804 100644 --- 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