* [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
@ 2014-03-02 2:29 Sun He
2014-03-03 7:41 ` Eric Sunshine
0 siblings, 1 reply; 3+ messages in thread
From: Sun He @ 2014-03-02 2:29 UTC (permalink / raw)
To: git; +Cc: Sun He
Signed-off-by: Sun He <sunheehnus@gmail.com>
Helped-by: Eric Sunshine <sunshine@sunshineco.com>
Helped-by: Michael Haggerty <mhagger@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));
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
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
2014-03-02 2:29 [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction Sun He
@ 2014-03-03 7:41 ` Eric Sunshine
2014-03-03 9:27 ` He Sun
0 siblings, 1 reply; 3+ messages in thread
From: Eric Sunshine @ 2014-03-03 7:41 UTC (permalink / raw)
To: Sun He; +Cc: Git List
On Sat, Mar 1, 2014 at 9:29 PM, Sun He <sunheehnus@gmail.com> wrote:
> Signed-off-by: Sun He <sunheehnus@gmail.com>
> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
> Helped-by: Michael Haggerty <mhagger@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 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
^ permalink raw reply [flat|nested] 3+ messages in thread
* Re: [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction
2014-03-03 7:41 ` Eric Sunshine
@ 2014-03-03 9:27 ` He Sun
0 siblings, 0 replies; 3+ messages in thread
From: He Sun @ 2014-03-03 9:27 UTC (permalink / raw)
To: Eric Sunshine, git
2014-03-03 15:41 GMT+08:00 Eric Sunshine <sunshine@sunshineco.com>:
> On Sat, Mar 1, 2014 at 9:29 PM, Sun He <sunheehnus@gmail.com> wrote:
>> Signed-off-by: Sun He <sunheehnus@gmail.com>
>> Helped-by: Eric Sunshine <sunshine@sunshineco.com>
>> Helped-by: Michael Haggerty <mhagger@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 **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
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2014-03-03 9:27 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-03-02 2:29 [PATCH v2] finish_tmp_packfile():use strbuf for pathname construction Sun He
2014-03-03 7:41 ` Eric Sunshine
2014-03-03 9:27 ` He Sun
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).