undef/git-commit-vandalism
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

midx.c: write MIDX filenames to strbuf

Browse Source 
To ask for the name of a MIDX and its corresponding .rev file, callers
invoke get_midx_filename() and get_midx_rev_filename(), respectively.
These both invoke xstrfmt(), allocating a chunk of memory which must be
freed later on.

This makes callers in pack-bitmap.c somewhat awkward. Specifically,
midx_bitmap_filename(), which is implemented like:

    return xstrfmt("%s-%s.bitmap",
                   get_midx_filename(midx->object_dir),
                   hash_to_hex(get_midx_checksum(midx)));

this leaks the second argument to xstrfmt(), which itself was allocated
with xstrfmt(). This caller could assign both the result of
get_midx_filename() and the outer xstrfmt() to a temporary variable,
remembering to free() the former before returning. But that involves a
wasteful copy.

Instead, get_midx_filename() and get_midx_rev_filename() take a strbuf
as an output parameter. This way midx_bitmap_filename() can manipulate
and pass around a temporary buffer which it detaches back to its caller.

That allows us to implement the function without copying or open-coding
get_midx_filename() in a way that doesn't leak.

Update the other callers of get_midx_filename() and
get_midx_rev_filename() accordingly.

Signed-off-by: Taylor Blau <me@ttaylorr.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
This commit is contained in:
Taylor Blau 2021-10-26 17:01:21 -04:00 committed by Junio C Hamano
parent ee4a1d63d7
commit 60980aed78
4 changed files with 49 additions and 37 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

59
midx.c
View File

@ -57,15 +57,15 @@ const unsigned char *get_midx_checksum(struct multi_pack_index *m)
return m->data + m->data_len - the_hash_algo->rawsz; return m->data + m->data_len - the_hash_algo->rawsz;
} }
char *get_midx_filename(const char *object_dir) void get_midx_filename(struct strbuf *out, const char *object_dir)
{ {
return xstrfmt("%s/pack/multi-pack-index", object_dir); strbuf_addf(out, "%s/pack/multi-pack-index", object_dir);
} }
char *get_midx_rev_filename(struct multi_pack_index *m) void get_midx_rev_filename(struct strbuf *out, struct multi_pack_index *m)
{ {
return xstrfmt("%s/pack/multi-pack-index-%s.rev", get_midx_filename(out, m->object_dir);
m->object_dir, hash_to_hex(get_midx_checksum(m))); strbuf_addf(out, "-%s.rev", hash_to_hex(get_midx_checksum(m)));
} }
static int midx_read_oid_fanout(const unsigned char *chunk_start, static int midx_read_oid_fanout(const unsigned char *chunk_start,
@ -89,28 +89,30 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
size_t midx_size; size_t midx_size;
void *midx_map = NULL; void *midx_map = NULL;
uint32_t hash_version; uint32_t hash_version;
char *midx_name = get_midx_filename(object_dir); struct strbuf midx_name = STRBUF_INIT;
uint32_t i; uint32_t i;
const char *cur_pack_name; const char *cur_pack_name;
struct chunkfile *cf = NULL; struct chunkfile *cf = NULL;
fd = git_open(midx_name); get_midx_filename(&midx_name, object_dir);
fd = git_open(midx_name.buf);
if (fd < 0) if (fd < 0)
goto cleanup_fail; goto cleanup_fail;
if (fstat(fd, &st)) { if (fstat(fd, &st)) {
error_errno(_("failed to read %s"), midx_name); error_errno(_("failed to read %s"), midx_name.buf);
goto cleanup_fail; goto cleanup_fail;
} }
midx_size = xsize_t(st.st_size); midx_size = xsize_t(st.st_size);
if (midx_size < MIDX_MIN_SIZE) { if (midx_size < MIDX_MIN_SIZE) {
error(_("multi-pack-index file %s is too small"), midx_name); error(_("multi-pack-index file %s is too small"), midx_name.buf);
goto cleanup_fail; goto cleanup_fail;
} }
FREE_AND_NULL(midx_name); strbuf_release(&midx_name);
midx_map = xmmap(NULL, midx_size, PROT_READ, MAP_PRIVATE, fd, 0); midx_map = xmmap(NULL, midx_size, PROT_READ, MAP_PRIVATE, fd, 0);
close(fd); close(fd);
@ -184,7 +186,7 @@ struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local
cleanup_fail: cleanup_fail:
free(m); free(m);
free(midx_name); strbuf_release(&midx_name);
free_chunkfile(cf); free_chunkfile(cf);
if (midx_map) if (midx_map)
munmap(midx_map, midx_size); munmap(midx_map, midx_size);
@ -1115,7 +1117,7 @@ static int write_midx_internal(const char *object_dir,
const char *refs_snapshot, const char *refs_snapshot,
unsigned flags) unsigned flags)
{ {
char *midx_name; struct strbuf midx_name = STRBUF_INIT;
unsigned char midx_hash[GIT_MAX_RAWSZ]; unsigned char midx_hash[GIT_MAX_RAWSZ];
uint32_t i; uint32_t i;
struct hashfile *f = NULL; struct hashfile *f = NULL;
@ -1130,10 +1132,10 @@ static int write_midx_internal(const char *object_dir,
/* Ensure the given object_dir is local, or a known alternate. */ /* Ensure the given object_dir is local, or a known alternate. */
find_odb(the_repository, object_dir); find_odb(the_repository, object_dir);
midx_name = get_midx_filename(object_dir); get_midx_filename(&midx_name, object_dir);
if (safe_create_leading_directories(midx_name)) if (safe_create_leading_directories(midx_name.buf))
die_errno(_("unable to create leading directories of %s"), die_errno(_("unable to create leading directories of %s"),
midx_name); midx_name.buf);
if (!packs_to_include) { if (!packs_to_include) {
/* /*
@ -1367,7 +1369,7 @@ static int write_midx_internal(const char *object_dir,
pack_name_concat_len += MIDX_CHUNK_ALIGNMENT - pack_name_concat_len += MIDX_CHUNK_ALIGNMENT -
(pack_name_concat_len % MIDX_CHUNK_ALIGNMENT); (pack_name_concat_len % MIDX_CHUNK_ALIGNMENT);
hold_lock_file_for_update(&lk, midx_name, LOCK_DIE_ON_ERROR); hold_lock_file_for_update(&lk, midx_name.buf, LOCK_DIE_ON_ERROR);
f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk)); f = hashfd(get_lock_file_fd(&lk), get_lock_file_path(&lk));
if (ctx.nr - dropped_packs == 0) { if (ctx.nr - dropped_packs == 0) {
@ -1404,9 +1406,9 @@ static int write_midx_internal(const char *object_dir,
ctx.pack_order = midx_pack_order(&ctx); ctx.pack_order = midx_pack_order(&ctx);
if (flags & MIDX_WRITE_REV_INDEX) if (flags & MIDX_WRITE_REV_INDEX)
write_midx_reverse_index(midx_name, midx_hash, &ctx); write_midx_reverse_index(midx_name.buf, midx_hash, &ctx);
if (flags & MIDX_WRITE_BITMAP) { if (flags & MIDX_WRITE_BITMAP) {
if (write_midx_bitmap(midx_name, midx_hash, &ctx, if (write_midx_bitmap(midx_name.buf, midx_hash, &ctx,
refs_snapshot, flags) < 0) { refs_snapshot, flags) < 0) {
error(_("could not write multi-pack bitmap")); error(_("could not write multi-pack bitmap"));
result = 1; result = 1;
@ -1435,7 +1437,7 @@ cleanup:
free(ctx.entries); free(ctx.entries);
free(ctx.pack_perm); free(ctx.pack_perm);
free(ctx.pack_order); free(ctx.pack_order);
free(midx_name); strbuf_release(&midx_name);
return result; return result;
} }
@ -1499,20 +1501,22 @@ static void clear_midx_files_ext(const char *object_dir, const char *ext,
void clear_midx_file(struct repository *r) void clear_midx_file(struct repository *r)
{ {
char *midx = get_midx_filename(r->objects->odb->path); struct strbuf midx = STRBUF_INIT;
get_midx_filename(&midx, r->objects->odb->path);
if (r->objects && r->objects->multi_pack_index) { if (r->objects && r->objects->multi_pack_index) {
close_midx(r->objects->multi_pack_index); close_midx(r->objects->multi_pack_index);
r->objects->multi_pack_index = NULL; r->objects->multi_pack_index = NULL;
} }
if (remove_path(midx)) if (remove_path(midx.buf))
die(_("failed to clear multi-pack-index at %s"), midx); die(_("failed to clear multi-pack-index at %s"), midx.buf);
clear_midx_files_ext(r->objects->odb->path, ".bitmap", NULL); clear_midx_files_ext(r->objects->odb->path, ".bitmap", NULL);
clear_midx_files_ext(r->objects->odb->path, ".rev", NULL); clear_midx_files_ext(r->objects->odb->path, ".rev", NULL);
free(midx); strbuf_release(&midx);
} }
static int verify_midx_error; static int verify_midx_error;
@ -1565,12 +1569,15 @@ int verify_midx_file(struct repository *r, const char *object_dir, unsigned flag
if (!m) { if (!m) {
int result = 0; int result = 0;
struct stat sb; struct stat sb;
char *filename = get_midx_filename(object_dir); struct strbuf filename = STRBUF_INIT;
if (!stat(filename, &sb)) {
get_midx_filename(&filename, object_dir);
if (!stat(filename.buf, &sb)) {
error(_("multi-pack-index file exists, but failed to parse")); error(_("multi-pack-index file exists, but failed to parse"));
result = 1; result = 1;
} }
free(filename); strbuf_release(&filename);
return result; return result;
} }

4
midx.h
View File

@ -48,8 +48,8 @@ struct multi_pack_index {
#define MIDX_WRITE_BITMAP_HASH_CACHE (1 << 3) #define MIDX_WRITE_BITMAP_HASH_CACHE (1 << 3)
const unsigned char *get_midx_checksum(struct multi_pack_index *m); const unsigned char *get_midx_checksum(struct multi_pack_index *m);
char *get_midx_filename(const char *object_dir); void get_midx_filename(struct strbuf *out, const char *object_dir);
char *get_midx_rev_filename(struct multi_pack_index *m); void get_midx_rev_filename(struct strbuf *out, struct multi_pack_index *m);
struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local); struct multi_pack_index *load_multi_pack_index(const char *object_dir, int local);
int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id); int prepare_midx_pack(struct repository *r, struct multi_pack_index *m, uint32_t pack_int_id);

15
pack-bitmap.c
View File

@ -292,9 +292,12 @@ static int load_bitmap_entries_v1(struct bitmap_index *index)
char *midx_bitmap_filename(struct multi_pack_index *midx) char *midx_bitmap_filename(struct multi_pack_index *midx)
{ {
return xstrfmt("%s-%s.bitmap", struct strbuf buf = STRBUF_INIT;
get_midx_filename(midx->object_dir),
hash_to_hex(get_midx_checksum(midx))); get_midx_filename(&buf, midx->object_dir);
strbuf_addf(&buf, "-%s.bitmap", hash_to_hex(get_midx_checksum(midx)));
return strbuf_detach(&buf, NULL);
} }
char *pack_bitmap_filename(struct packed_git *p) char *pack_bitmap_filename(struct packed_git *p)
@ -324,10 +327,12 @@ static int open_midx_bitmap_1(struct bitmap_index *bitmap_git,
} }
if (bitmap_git->pack || bitmap_git->midx) { if (bitmap_git->pack || bitmap_git->midx) {
struct strbuf buf = STRBUF_INIT;
get_midx_filename(&buf, midx->object_dir);
/* ignore extra bitmap file; we can only handle one */ /* ignore extra bitmap file; we can only handle one */
warning("ignoring extra bitmap file: %s", warning("ignoring extra bitmap file: %s", buf.buf);
get_midx_filename(midx->object_dir));
close(fd); close(fd);
strbuf_release(&buf);
return -1; return -1;
} }

8
pack-revindex.c
View File

@ -296,14 +296,14 @@ int load_pack_revindex(struct packed_git *p)
int load_midx_revindex(struct multi_pack_index *m) int load_midx_revindex(struct multi_pack_index *m)
{ {
char *revindex_name; struct strbuf revindex_name = STRBUF_INIT;
int ret; int ret;
if (m->revindex_data) if (m->revindex_data)
return 0; return 0;
revindex_name = get_midx_rev_filename(m); get_midx_rev_filename(&revindex_name, m);
ret = load_revindex_from_disk(revindex_name, ret = load_revindex_from_disk(revindex_name.buf,
m->num_objects, m->num_objects,
&m->revindex_map, &m->revindex_map,
&m->revindex_len); &m->revindex_len);
@ -313,7 +313,7 @@ int load_midx_revindex(struct multi_pack_index *m)
m->revindex_data = (const uint32_t *)((const char *)m->revindex_map + RIDX_HEADER_SIZE); m->revindex_data = (const uint32_t *)((const char *)m->revindex_map + RIDX_HEADER_SIZE);
cleanup: cleanup:
free(revindex_name); strbuf_release(&revindex_name);
return ret; return ret;
} }