Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
On Sat, Mar 1, 2014 at 9:29 PM, Sun He wrote: > Signed-off-by: Sun He > Helped-by: Eric Sunshine > Helped-by: Michael Haggerty > --- > > 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 "-.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); > } >
Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
2014-03-03 15:41 GMT+08:00 Eric Sunshine : > On Sat, Mar 1, 2014 at 9:29 PM, Sun He wrote: >> Signed-off-by: Sun He >> Helped-by: Eric Sunshine >> Helped-by: Michael Haggerty >> --- >> >> 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 "-.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/pa