From 92e0b67930f350a40575954f4638cbc71fee57d2 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 13:31:30 +0100 Subject: [PATCH 1/6] buffer: Do not `put` anything if len is 0 --- src/buffer.c | 11 +++++++---- tests/core/buffer.c | 5 ++--- 2 files changed, 9 insertions(+), 7 deletions(-) diff --git a/src/buffer.c b/src/buffer.c index e9c420e16..7744d8f49 100644 --- a/src/buffer.c +++ b/src/buffer.c @@ -176,10 +176,13 @@ int git_buf_putcn(git_buf *buf, char c, size_t len) int git_buf_put(git_buf *buf, const char *data, size_t len) { - ENSURE_SIZE(buf, buf->size + len + 1); - memmove(buf->ptr + buf->size, data, len); - buf->size += len; - buf->ptr[buf->size] = '\0'; + if (len) { + assert(data); + ENSURE_SIZE(buf, buf->size + len + 1); + memmove(buf->ptr + buf->size, data, len); + buf->size += len; + buf->ptr[buf->size] = '\0'; + } return 0; } diff --git a/tests/core/buffer.c b/tests/core/buffer.c index 641fed630..87dec4607 100644 --- a/tests/core/buffer.c +++ b/tests/core/buffer.c @@ -281,11 +281,10 @@ check_buf_append_abc( /* more variations on append tests */ void test_core_buffer__5(void) { - check_buf_append("", "", "", 0, 8); - check_buf_append("a", "", "a", 1, 8); + check_buf_append("", "", "", 0, 0); + check_buf_append("a", "", "a", 1, 0); check_buf_append("", "a", "a", 1, 8); check_buf_append("", "a", "a", 1, 8); - check_buf_append("a", "", "a", 1, 8); check_buf_append("a", "b", "ab", 2, 8); check_buf_append("", "abcdefgh", "abcdefgh", 8, 16); check_buf_append("abcdefgh", "", "abcdefgh", 8, 16); From 72d0024134a532374e7fa17ae9e5b6b021ffe324 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 13:32:21 +0100 Subject: [PATCH 2/6] attr_file: Do not assume ODB data is NULL-terminated That's a bad assumption to make, even though right now it holds (because of the way we've implemented decompression of packfiles), this may change in the future, given that ODB objects can be binary data. Furthermore, the ODB object can return a NULL pointer if the object is empty. Copying the NULL pointer to the strbuf lets us handle it like an empty string. Again, the NULL pointer is valid behavior because you're supposed to check the *size* of the object before working on it. --- src/attr_file.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/attr_file.c b/src/attr_file.c index e3692cee9..b3efeefd7 100644 --- a/src/attr_file.c +++ b/src/attr_file.c @@ -103,7 +103,6 @@ int git_attr_file__load( int error = 0; git_blob *blob = NULL; git_buf content = GIT_BUF_INIT; - const char *data = NULL; git_attr_file *file; struct stat st; @@ -120,7 +119,9 @@ int git_attr_file__load( (error = git_blob_lookup(&blob, repo, &id)) < 0) return error; - data = git_blob_rawcontent(blob); + /* Do not assume that data straight from the ODB is NULL-terminated; + * copy the contents of a file to a buffer to work on */ + git_buf_put(&content, git_blob_rawcontent(blob), git_blob_rawsize(blob)); break; } case GIT_ATTR_FILE__FROM_FILE: { @@ -143,7 +144,6 @@ int git_attr_file__load( if (error < 0) return GIT_ENOTFOUND; - data = content.ptr; break; } default: @@ -154,7 +154,7 @@ int git_attr_file__load( if ((error = git_attr_file__new(&file, entry, source)) < 0) goto cleanup; - if (parser && (error = parser(repo, file, data)) < 0) { + if (parser && (error = parser(repo, file, git_buf_cstr(&content))) < 0) { git_attr_file__free(file); goto cleanup; } From 1ba48b7caf8e2de010b8c0038860b90be0692274 Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 17:19:41 +0100 Subject: [PATCH 3/6] notes: Do not assume blob contents are NULL-terminated --- src/notes.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/notes.c b/src/notes.c index 046a91614..9b0d4d5ac 100644 --- a/src/notes.c +++ b/src/notes.c @@ -313,6 +313,7 @@ static int note_new( git_blob *blob) { git_note *note = NULL; + git_buf note_contents = GIT_BUF_INIT; note = (git_note *)git__malloc(sizeof(git_note)); GITERR_CHECK_ALLOC(note); @@ -323,11 +324,10 @@ static int note_new( git_signature_dup(¬e->committer, git_commit_committer(commit)) < 0) return -1; - note->message = git__strdup((char *)git_blob_rawcontent(blob)); - GITERR_CHECK_ALLOC(note->message); + git_buf_put(¬e_contents, git_blob_rawcontent(blob), git_blob_rawsize(blob)); + note->message = git_buf_detach(¬e_contents); *out = note; - return 0; } From 2e1e0f108f324719537bddd537099798e7d7449a Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 17:24:55 +0100 Subject: [PATCH 4/6] blame: Do not assume blob contents are NULL-terminated --- examples/blame.c | 14 ++++++++------ 1 file changed, 8 insertions(+), 6 deletions(-) diff --git a/examples/blame.c b/examples/blame.c index 9d38f25a4..b126c0d33 100644 --- a/examples/blame.c +++ b/examples/blame.c @@ -38,6 +38,7 @@ static void parse_opts(struct opts *o, int argc, char *argv[]); int main(int argc, char *argv[]) { int i, line, break_on_null_hunk; + size_t rawsize; char spec[1024] = {0}; struct opts o = {0}; const char *rawdata; @@ -94,23 +95,24 @@ int main(int argc, char *argv[]) git_object_free(obj); rawdata = git_blob_rawcontent(blob); + rawsize = git_blob_rawsize(blob); /** Produce the output. */ line = 1; i = 0; break_on_null_hunk = 0; - while (i < git_blob_rawsize(blob)) { - const char *eol = strchr(rawdata+i, '\n'); + while (i < rawsize) { + const char *eol = memchr(rawdata + i, '\n', rawsize - i); char oid[10] = {0}; const git_blame_hunk *hunk = git_blame_get_hunk_byline(blame, line); - if (break_on_null_hunk && !hunk) break; + if (break_on_null_hunk && !hunk) + break; if (hunk) { char sig[128] = {0}; break_on_null_hunk = 1; - git_oid_tostr(oid, 10, &hunk->final_commit_id); snprintf(sig, 30, "%s <%s>", hunk->final_signature->name, hunk->final_signature->email); @@ -118,8 +120,8 @@ int main(int argc, char *argv[]) oid, sig, line, - (int)(eol-rawdata-i), - rawdata+i); + (int)(eol - rawdata - i), + rawdata + i); } i = (int)(eol - rawdata + 1); From b7fb71e39cd48b61300647eec1e7dabab43da5af Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 17:38:55 +0100 Subject: [PATCH 5/6] notes: Use `git__strndup` --- src/notes.c | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/src/notes.c b/src/notes.c index 9b0d4d5ac..4b15925fd 100644 --- a/src/notes.c +++ b/src/notes.c @@ -313,7 +313,6 @@ static int note_new( git_blob *blob) { git_note *note = NULL; - git_buf note_contents = GIT_BUF_INIT; note = (git_note *)git__malloc(sizeof(git_note)); GITERR_CHECK_ALLOC(note); @@ -324,8 +323,8 @@ static int note_new( git_signature_dup(¬e->committer, git_commit_committer(commit)) < 0) return -1; - git_buf_put(¬e_contents, git_blob_rawcontent(blob), git_blob_rawsize(blob)); - note->message = git_buf_detach(¬e_contents); + note->message = git__strndup(git_blob_rawcontent(blob), git_blob_rawsize(blob)); + GITERR_CHECK_ALLOC(note->message); *out = note; return 0; From 24cce2398f893b77f183425fffd957daa3300c5a Mon Sep 17 00:00:00 2001 From: Vicent Marti Date: Fri, 21 Nov 2014 18:09:57 +0100 Subject: [PATCH 6/6] text: Null-terminate a string if we've been gouging it --- src/buf_text.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/buf_text.c b/src/buf_text.c index 8d2b141b2..cead599f4 100644 --- a/src/buf_text.c +++ b/src/buf_text.c @@ -143,6 +143,7 @@ int git_buf_text_lf_to_crlf(git_buf *tgt, const git_buf *src) tgt->ptr[tgt->size++] = '\n'; } + tgt->ptr[tgt->size] = '\0'; return git_buf_put(tgt, scan, end - scan); }