Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2014-03-03 15:41 GMT+08:00 Eric Sunshine <sunshine@xxxxxxxxxxxxxx>:
> On Sat, Mar 1, 2014 at 9:29 PM, Sun He <sunheehnus@xxxxxxxxx> wrote:
>> Signed-off-by: Sun He <sunheehnus@xxxxxxxxx>
>> Helped-by: Eric Sunshine <sunshine@xxxxxxxxxxxxxx>
>> Helped-by: Michael Haggerty <mhagger@xxxxxxxxxxxx>
>> ---
>>
>> 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 **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 12d9516..3223f5a 100644
>> --- 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
>> --
>> 1.9.0.138.g2de3478.dirty
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]