Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
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>
  • Loading branch information
Sun He authored and Junio C Hamano committed Mar 3, 2014
1 parent 2156a98 commit 5889271
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 21 deletions.
15 changes: 6 additions & 9 deletions builtin/pack-objects.c
Expand Up @@ -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
Expand All @@ -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);

Expand All @@ -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));
}
Expand Down
8 changes: 5 additions & 3 deletions bulk-checkin.c
Expand Up @@ -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;

Expand All @@ -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)
Expand All @@ -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++)
Expand All @@ -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();
}
Expand Down
18 changes: 10 additions & 8 deletions pack-write.c
Expand Up @@ -336,15 +336,15 @@ 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,
struct pack_idx_option *pack_idx_opts,
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");
Expand All @@ -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);
}
2 changes: 1 addition & 1 deletion pack.h
Expand Up @@ -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

0 comments on commit 5889271

Please sign in to comment.