mirror of
https://github.com/git/git.git
synced 2024-11-18 20:13:59 +01:00
Merge branch 'kg/packed-ref-cache-fix'
Avoid mmapping small files while using packed refs (especially ones with zero size, which would cause later munmap() to fail). * kg/packed-ref-cache-fix: packed_ref_cache: don't use mmap() for small files load_contents(): don't try to mmap an empty file packed_ref_iterator_begin(): make optimization more general find_reference_location(): make function safe for empty snapshots create_snapshot(): use `xmemdupz()` rather than a strbuf struct snapshot: store `start` rather than `header_len`
This commit is contained in:
commit
9db22910f7
@ -68,17 +68,21 @@ struct snapshot {
|
||||
int mmapped;
|
||||
|
||||
/*
|
||||
* The contents of the `packed-refs` file. If the file was
|
||||
* already sorted, this points at the mmapped contents of the
|
||||
* file. If not, this points at heap-allocated memory
|
||||
* containing the contents, sorted. If there were no contents
|
||||
* (e.g., because the file didn't exist), `buf` and `eof` are
|
||||
* both NULL.
|
||||
* The contents of the `packed-refs` file:
|
||||
*
|
||||
* - buf -- a pointer to the start of the memory
|
||||
* - start -- a pointer to the first byte of actual references
|
||||
* (i.e., after the header line, if one is present)
|
||||
* - eof -- a pointer just past the end of the reference
|
||||
* contents
|
||||
*
|
||||
* If the `packed-refs` file was already sorted, `buf` points
|
||||
* at the mmapped contents of the file. If not, it points at
|
||||
* heap-allocated memory containing the contents, sorted. If
|
||||
* there were no contents (e.g., because the file didn't
|
||||
* exist), `buf`, `start`, and `eof` are all NULL.
|
||||
*/
|
||||
char *buf, *eof;
|
||||
|
||||
/* The size of the header line, if any; otherwise, 0: */
|
||||
size_t header_len;
|
||||
char *buf, *start, *eof;
|
||||
|
||||
/*
|
||||
* What is the peeled state of the `packed-refs` file that
|
||||
@ -169,8 +173,7 @@ static void clear_snapshot_buffer(struct snapshot *snapshot)
|
||||
} else {
|
||||
free(snapshot->buf);
|
||||
}
|
||||
snapshot->buf = snapshot->eof = NULL;
|
||||
snapshot->header_len = 0;
|
||||
snapshot->buf = snapshot->start = snapshot->eof = NULL;
|
||||
}
|
||||
|
||||
/*
|
||||
@ -319,13 +322,14 @@ static void sort_snapshot(struct snapshot *snapshot)
|
||||
size_t len, i;
|
||||
char *new_buffer, *dst;
|
||||
|
||||
pos = snapshot->buf + snapshot->header_len;
|
||||
pos = snapshot->start;
|
||||
eof = snapshot->eof;
|
||||
len = eof - pos;
|
||||
|
||||
if (!len)
|
||||
if (pos == eof)
|
||||
return;
|
||||
|
||||
len = eof - pos;
|
||||
|
||||
/*
|
||||
* Initialize records based on a crude estimate of the number
|
||||
* of references in the file (we'll grow it below if needed):
|
||||
@ -391,9 +395,8 @@ static void sort_snapshot(struct snapshot *snapshot)
|
||||
* place:
|
||||
*/
|
||||
clear_snapshot_buffer(snapshot);
|
||||
snapshot->buf = new_buffer;
|
||||
snapshot->buf = snapshot->start = new_buffer;
|
||||
snapshot->eof = new_buffer + len;
|
||||
snapshot->header_len = 0;
|
||||
|
||||
cleanup:
|
||||
free(records);
|
||||
@ -442,23 +445,26 @@ static const char *find_end_of_record(const char *p, const char *end)
|
||||
*/
|
||||
static void verify_buffer_safe(struct snapshot *snapshot)
|
||||
{
|
||||
const char *buf = snapshot->buf + snapshot->header_len;
|
||||
const char *start = snapshot->start;
|
||||
const char *eof = snapshot->eof;
|
||||
const char *last_line;
|
||||
|
||||
if (buf == eof)
|
||||
if (start == eof)
|
||||
return;
|
||||
|
||||
last_line = find_start_of_record(buf, eof - 1);
|
||||
last_line = find_start_of_record(start, eof - 1);
|
||||
if (*(eof - 1) != '\n' || eof - last_line < GIT_SHA1_HEXSZ + 2)
|
||||
die_invalid_line(snapshot->refs->path,
|
||||
last_line, eof - last_line);
|
||||
}
|
||||
|
||||
#define SMALL_FILE_SIZE (32*1024)
|
||||
|
||||
/*
|
||||
* Depending on `mmap_strategy`, either mmap or read the contents of
|
||||
* the `packed-refs` file into the snapshot. Return 1 if the file
|
||||
* existed and was read, or 0 if the file was absent. Die on errors.
|
||||
* existed and was read, or 0 if the file was absent or empty. Die on
|
||||
* errors.
|
||||
*/
|
||||
static int load_contents(struct snapshot *snapshot)
|
||||
{
|
||||
@ -489,24 +495,23 @@ static int load_contents(struct snapshot *snapshot)
|
||||
die_errno("couldn't stat %s", snapshot->refs->path);
|
||||
size = xsize_t(st.st_size);
|
||||
|
||||
switch (mmap_strategy) {
|
||||
case MMAP_NONE:
|
||||
if (!size) {
|
||||
return 0;
|
||||
} else if (mmap_strategy == MMAP_NONE || size <= SMALL_FILE_SIZE) {
|
||||
snapshot->buf = xmalloc(size);
|
||||
bytes_read = read_in_full(fd, snapshot->buf, size);
|
||||
if (bytes_read < 0 || bytes_read != size)
|
||||
die_errno("couldn't read %s", snapshot->refs->path);
|
||||
snapshot->eof = snapshot->buf + size;
|
||||
snapshot->mmapped = 0;
|
||||
break;
|
||||
case MMAP_TEMPORARY:
|
||||
case MMAP_OK:
|
||||
} else {
|
||||
snapshot->buf = xmmap(NULL, size, PROT_READ, MAP_PRIVATE, fd, 0);
|
||||
snapshot->eof = snapshot->buf + size;
|
||||
snapshot->mmapped = 1;
|
||||
break;
|
||||
}
|
||||
close(fd);
|
||||
|
||||
snapshot->start = snapshot->buf;
|
||||
snapshot->eof = snapshot->buf + size;
|
||||
|
||||
return 1;
|
||||
}
|
||||
|
||||
@ -515,9 +520,11 @@ static int load_contents(struct snapshot *snapshot)
|
||||
* `refname` starts. If `mustexist` is true and the reference doesn't
|
||||
* exist, then return NULL. If `mustexist` is false and the reference
|
||||
* doesn't exist, then return the point where that reference would be
|
||||
* inserted. In the latter mode, `refname` doesn't have to be a proper
|
||||
* reference name; for example, one could search for "refs/replace/"
|
||||
* to find the start of any replace references.
|
||||
* inserted, or `snapshot->eof` (which might be NULL) if it would be
|
||||
* inserted at the end of the file. In the latter mode, `refname`
|
||||
* doesn't have to be a proper reference name; for example, one could
|
||||
* search for "refs/replace/" to find the start of any replace
|
||||
* references.
|
||||
*
|
||||
* The record is sought using a binary search, so `snapshot->buf` must
|
||||
* be sorted.
|
||||
@ -539,7 +546,7 @@ static const char *find_reference_location(struct snapshot *snapshot,
|
||||
* preceding records all have reference names that come
|
||||
* *before* `refname`.
|
||||
*/
|
||||
const char *lo = snapshot->buf + snapshot->header_len;
|
||||
const char *lo = snapshot->start;
|
||||
|
||||
/*
|
||||
* A pointer to a the first character of a record whose
|
||||
@ -547,7 +554,7 @@ static const char *find_reference_location(struct snapshot *snapshot,
|
||||
*/
|
||||
const char *hi = snapshot->eof;
|
||||
|
||||
while (lo < hi) {
|
||||
while (lo != hi) {
|
||||
const char *mid, *rec;
|
||||
int cmp;
|
||||
|
||||
@ -616,9 +623,7 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
|
||||
|
||||
/* If the file has a header line, process it: */
|
||||
if (snapshot->buf < snapshot->eof && *snapshot->buf == '#') {
|
||||
struct strbuf tmp = STRBUF_INIT;
|
||||
char *p;
|
||||
const char *eol;
|
||||
char *tmp, *p, *eol;
|
||||
struct string_list traits = STRING_LIST_INIT_NODUP;
|
||||
|
||||
eol = memchr(snapshot->buf, '\n',
|
||||
@ -628,9 +633,9 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
|
||||
snapshot->buf,
|
||||
snapshot->eof - snapshot->buf);
|
||||
|
||||
strbuf_add(&tmp, snapshot->buf, eol - snapshot->buf);
|
||||
tmp = xmemdupz(snapshot->buf, eol - snapshot->buf);
|
||||
|
||||
if (!skip_prefix(tmp.buf, "# pack-refs with:", (const char **)&p))
|
||||
if (!skip_prefix(tmp, "# pack-refs with:", (const char **)&p))
|
||||
die_invalid_line(refs->path,
|
||||
snapshot->buf,
|
||||
snapshot->eof - snapshot->buf);
|
||||
@ -647,10 +652,10 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
|
||||
/* perhaps other traits later as well */
|
||||
|
||||
/* The "+ 1" is for the LF character. */
|
||||
snapshot->header_len = eol + 1 - snapshot->buf;
|
||||
snapshot->start = eol + 1;
|
||||
|
||||
string_list_clear(&traits, 0);
|
||||
strbuf_release(&tmp);
|
||||
free(tmp);
|
||||
}
|
||||
|
||||
verify_buffer_safe(snapshot);
|
||||
@ -671,13 +676,12 @@ static struct snapshot *create_snapshot(struct packed_ref_store *refs)
|
||||
* We don't want to leave the file mmapped, so we are
|
||||
* forced to make a copy now:
|
||||
*/
|
||||
size_t size = snapshot->eof -
|
||||
(snapshot->buf + snapshot->header_len);
|
||||
size_t size = snapshot->eof - snapshot->start;
|
||||
char *buf_copy = xmalloc(size);
|
||||
|
||||
memcpy(buf_copy, snapshot->buf + snapshot->header_len, size);
|
||||
memcpy(buf_copy, snapshot->start, size);
|
||||
clear_snapshot_buffer(snapshot);
|
||||
snapshot->buf = buf_copy;
|
||||
snapshot->buf = snapshot->start = buf_copy;
|
||||
snapshot->eof = buf_copy + size;
|
||||
}
|
||||
|
||||
@ -924,7 +928,12 @@ static struct ref_iterator *packed_ref_iterator_begin(
|
||||
*/
|
||||
snapshot = get_snapshot(refs);
|
||||
|
||||
if (!snapshot->buf)
|
||||
if (prefix && *prefix)
|
||||
start = find_reference_location(snapshot, prefix, 0);
|
||||
else
|
||||
start = snapshot->start;
|
||||
|
||||
if (start == snapshot->eof)
|
||||
return empty_ref_iterator_begin();
|
||||
|
||||
iter = xcalloc(1, sizeof(*iter));
|
||||
@ -934,11 +943,6 @@ static struct ref_iterator *packed_ref_iterator_begin(
|
||||
iter->snapshot = snapshot;
|
||||
acquire_snapshot(snapshot);
|
||||
|
||||
if (prefix && *prefix)
|
||||
start = find_reference_location(snapshot, prefix, 0);
|
||||
else
|
||||
start = snapshot->buf + snapshot->header_len;
|
||||
|
||||
iter->pos = start;
|
||||
iter->eof = snapshot->eof;
|
||||
strbuf_init(&iter->refname_buf, 0);
|
||||
|
Loading…
Reference in New Issue
Block a user