From 2f2575c002e971f28ac6eb7d4b71f6736b060e0f Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Tue, 22 Dec 2015 10:38:16 -0700 Subject: [PATCH 01/58] Updating http parser to accept a `+` in the schema --- deps/http-parser/http_parser.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/deps/http-parser/http_parser.c b/deps/http-parser/http_parser.c index 203530254..b793d7011 100644 --- a/deps/http-parser/http_parser.c +++ b/deps/http-parser/http_parser.c @@ -451,7 +451,7 @@ parse_url_char(enum state s, const char ch) break; case s_req_schema: - if (IS_ALPHA(ch)) { + if (IS_ALPHA(ch) || ch == '+') { return s; } From ff8e3f0e6b1fb74b768d7098d912bd0ddabf71ce Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Tue, 22 Dec 2015 10:38:31 -0700 Subject: [PATCH 02/58] Handle git+ssh:// and ssh+git:// protocols support --- src/transport.c | 2 ++ src/transports/ssh.c | 54 ++++++++++++++++++++++++++++---------------- 2 files changed, 37 insertions(+), 19 deletions(-) diff --git a/src/transport.c b/src/transport.c index 5c65c7c06..cf785bc88 100644 --- a/src/transport.c +++ b/src/transport.c @@ -35,6 +35,8 @@ static transport_definition transports[] = { { "file://", git_transport_local, NULL }, #ifdef GIT_SSH { "ssh://", git_transport_smart, &ssh_subtransport_definition }, + { "ssh+git://", git_transport_smart, &ssh_subtransport_definition }, + { "git+ssh://", git_transport_smart, &ssh_subtransport_definition }, #endif { NULL, 0, 0 } }; diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 35739abe3..60d210afb 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -21,7 +21,9 @@ #define OWNING_SUBTRANSPORT(s) ((ssh_subtransport *)(s)->parent.subtransport) -static const char prefix_ssh[] = "ssh://"; +static const char * ssh_prefixes[] = { "ssh://", "ssh+git://", "git+ssh://" }; +#define SSH_PREFIX_COUNT (sizeof(ssh_prefixes) / sizeof(ssh_prefixes[0])) + static const char cmd_uploadpack[] = "git-upload-pack"; static const char cmd_receivepack[] = "git-receive-pack"; @@ -64,16 +66,23 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url) char *repo; int len; - if (!git__prefixcmp(url, prefix_ssh)) { - url = url + strlen(prefix_ssh); - repo = strchr(url, '/'); - if (repo && repo[1] == '~') - ++repo; - } else { - repo = strchr(url, ':'); - if (repo) repo++; - } + size_t i = 0; + for (i = 0; i < SSH_PREFIX_COUNT; ++i) { + const char *p = ssh_prefixes[i]; + if (!git__prefixcmp(url, p)) { + url = url + strlen(p); + repo = strchr(url, '/'); + if (repo && repo[1] == '~') + ++repo; + + goto done; + } + } + repo = strchr(url, ':'); + if (repo) repo++; + +done: if (!repo) { giterr_set(GITERR_NET, "Malformed git protocol URL"); return -1; @@ -515,16 +524,23 @@ static int _git_ssh_setup_conn( s->session = NULL; s->channel = NULL; - if (!git__prefixcmp(url, prefix_ssh)) { - if ((error = gitno_extract_url_parts(&host, &port, &path, &user, &pass, url, default_port)) < 0) - goto done; - } else { - if ((error = git_ssh_extract_url_parts(&host, &user, url)) < 0) - goto done; - port = git__strdup(default_port); - GITERR_CHECK_ALLOC(port); - } + size_t i = 0; + for (i = 0; i < SSH_PREFIX_COUNT; ++i) { + const char *p = ssh_prefixes[i]; + if (!git__prefixcmp(url, p)) { + if ((error = gitno_extract_url_parts(&host, &port, &path, &user, &pass, url, default_port)) < 0) + goto done; + + goto post_extract; + } + } + if ((error = git_ssh_extract_url_parts(&host, &user, url)) < 0) + goto done; + port = git__strdup(default_port); + GITERR_CHECK_ALLOC(port); + +post_extract: if ((error = git_socket_stream_new(&s->io, host, port)) < 0 || (error = git_stream_connect(s->io)) < 0) goto done; From fa8b1a8822afecbf632efe98a35b8363c14053ca Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Tue, 22 Dec 2015 10:56:38 -0700 Subject: [PATCH 03/58] Adding spec coverage for ssh+git and git+ssh protocols --- tests/transport/register.c | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/tests/transport/register.c b/tests/transport/register.c index ea917d5d3..385ef0a4c 100644 --- a/tests/transport/register.c +++ b/tests/transport/register.c @@ -44,6 +44,8 @@ void test_transport_register__custom_transport_ssh(void) #ifndef GIT_SSH cl_git_fail_with(git_transport_new(&transport, NULL, "ssh://somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); #else cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); @@ -60,6 +62,8 @@ void test_transport_register__custom_transport_ssh(void) #ifndef GIT_SSH cl_git_fail_with(git_transport_new(&transport, NULL, "ssh://somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); #else cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); From 429155d516076d5bc22688ec79b865928948aad4 Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Mon, 28 Dec 2015 07:40:15 -0700 Subject: [PATCH 04/58] Updating change to http_parser to reflect PR for nodejs/http-parser The parser now also supports digits, '-' and '.'. https://github.com/nodejs/http-parser/pull/276 --- deps/http-parser/http_parser.c | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/deps/http-parser/http_parser.c b/deps/http-parser/http_parser.c index b793d7011..27bdd2081 100644 --- a/deps/http-parser/http_parser.c +++ b/deps/http-parser/http_parser.c @@ -99,7 +99,7 @@ do { \ FOR##_mark = NULL; \ } \ } while (0) - + /* Run the data callback FOR and consume the current byte */ #define CALLBACK_DATA(FOR) \ CALLBACK_DATA_(FOR, p - FOR##_mark, p - data + 1) @@ -444,6 +444,9 @@ parse_url_char(enum state s, const char ch) return s_req_path; } + /* The schema must start with an alpha character. After that, it may + * consist of digits, '+', '-' or '.', followed by a ':'. + */ if (IS_ALPHA(ch)) { return s_req_schema; } @@ -451,7 +454,7 @@ parse_url_char(enum state s, const char ch) break; case s_req_schema: - if (IS_ALPHA(ch) || ch == '+') { + if (IS_ALPHANUM(ch) || ch == '+' || ch == '-' || ch == '.') { return s; } From e44f6586ce41a8588438b5b291471e08feddbff5 Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Mon, 28 Dec 2015 07:43:24 -0700 Subject: [PATCH 05/58] Removing #define for SSH_PREFIX_COUNT and using ARRAY_SIZE instead Also moving var declarations to top of blocks to support bad old compilers --- src/transports/ssh.c | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/transports/ssh.c b/src/transports/ssh.c index 60d210afb..cfd573665 100644 --- a/src/transports/ssh.c +++ b/src/transports/ssh.c @@ -21,8 +21,7 @@ #define OWNING_SUBTRANSPORT(s) ((ssh_subtransport *)(s)->parent.subtransport) -static const char * ssh_prefixes[] = { "ssh://", "ssh+git://", "git+ssh://" }; -#define SSH_PREFIX_COUNT (sizeof(ssh_prefixes) / sizeof(ssh_prefixes[0])) +static const char *ssh_prefixes[] = { "ssh://", "ssh+git://", "git+ssh://" }; static const char cmd_uploadpack[] = "git-upload-pack"; static const char cmd_receivepack[] = "git-receive-pack"; @@ -65,9 +64,9 @@ static int gen_proto(git_buf *request, const char *cmd, const char *url) { char *repo; int len; + size_t i; - size_t i = 0; - for (i = 0; i < SSH_PREFIX_COUNT; ++i) { + for (i = 0; i < ARRAY_SIZE(ssh_prefixes); ++i) { const char *p = ssh_prefixes[i]; if (!git__prefixcmp(url, p)) { @@ -509,6 +508,7 @@ static int _git_ssh_setup_conn( char *host=NULL, *port=NULL, *path=NULL, *user=NULL, *pass=NULL; const char *default_port="22"; int auth_methods, error = 0; + size_t i; ssh_stream *s; git_cred *cred = NULL; LIBSSH2_SESSION* session=NULL; @@ -524,8 +524,7 @@ static int _git_ssh_setup_conn( s->session = NULL; s->channel = NULL; - size_t i = 0; - for (i = 0; i < SSH_PREFIX_COUNT; ++i) { + for (i = 0; i < ARRAY_SIZE(ssh_prefixes); ++i) { const char *p = ssh_prefixes[i]; if (!git__prefixcmp(url, p)) { From b8dc15f70e955b641e64ded29ca16c6c30ce7308 Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Mon, 28 Dec 2015 11:35:19 -0700 Subject: [PATCH 06/58] Adding test cases that actually test the functionality of the new transport ssh, ssh+git and git+ssh should all successfully build an SSH transport --- tests/transport/register.c | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/transport/register.c b/tests/transport/register.c index 385ef0a4c..67a2efd99 100644 --- a/tests/transport/register.c +++ b/tests/transport/register.c @@ -48,6 +48,9 @@ void test_transport_register__custom_transport_ssh(void) cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); #else + cl_git_pass(git_transport_new(&transport, NULL, "ssh://somehost:somepath")); + cl_git_pass(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath")); + cl_git_pass(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath")); cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); transport->free(transport); #endif @@ -66,6 +69,9 @@ void test_transport_register__custom_transport_ssh(void) cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); #else + cl_git_pass(git_transport_new(&transport, NULL, "ssh://somehost:somepath")); + cl_git_pass(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath")); + cl_git_pass(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath")); cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); transport->free(transport); #endif From 4a93a7fcc44a4ea12baca5b289bf89ae7cfb3b9c Mon Sep 17 00:00:00 2001 From: Chris Bargren Date: Mon, 28 Dec 2015 11:37:39 -0700 Subject: [PATCH 07/58] Tabs --- src/transport.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/transport.c b/src/transport.c index cf785bc88..327052fa3 100644 --- a/src/transport.c +++ b/src/transport.c @@ -35,8 +35,8 @@ static transport_definition transports[] = { { "file://", git_transport_local, NULL }, #ifdef GIT_SSH { "ssh://", git_transport_smart, &ssh_subtransport_definition }, - { "ssh+git://", git_transport_smart, &ssh_subtransport_definition }, - { "git+ssh://", git_transport_smart, &ssh_subtransport_definition }, + { "ssh+git://", git_transport_smart, &ssh_subtransport_definition }, + { "git+ssh://", git_transport_smart, &ssh_subtransport_definition }, #endif { NULL, 0, 0 } }; From 8ec3d88f585e14760d8acf8393d72d94e0c664b5 Mon Sep 17 00:00:00 2001 From: Yong Li Date: Thu, 24 Dec 2015 10:04:44 -0500 Subject: [PATCH 08/58] Avoid subtraction overflow in git_indexer_commit --- src/indexer.c | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 9aa092556..6e9af06a5 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -914,12 +914,17 @@ int git_indexer_commit(git_indexer *idx, git_transfer_progress *stats) git_filebuf index_file = {0}; void *packfile_trailer; + if (!idx->parsed_header) { + giterr_set(GITERR_INDEXER, "incomplete pack header"); + return -1; + } + if (git_hash_ctx_init(&ctx) < 0) return -1; /* Test for this before resolve_deltas(), as it plays with idx->off */ - if (idx->off < idx->pack->mwf.size - 20) { - giterr_set(GITERR_INDEXER, "Unexpected data at the end of the pack"); + if (idx->off + 20 < idx->pack->mwf.size) { + giterr_set(GITERR_INDEXER, "unexpected data at the end of the pack"); return -1; } From e3862c9fb21c892afd5b41aa91ff8680a8ccb9dd Mon Sep 17 00:00:00 2001 From: Chris Hescock Date: Mon, 11 Jan 2016 17:09:32 -0500 Subject: [PATCH 09/58] Buffer sideband packet data The inner packet may be split across multiple sideband packets. --- src/transports/smart_protocol.c | 59 +++++++++++++++++++++++---------- 1 file changed, 41 insertions(+), 18 deletions(-) diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 6363378ec..7d381b4cb 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -721,22 +721,20 @@ static int add_push_report_pkt(git_push *push, git_pkt *pkt) return 0; } -static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt) +static int add_push_report_sideband_pkt(git_push *push, git_buf *data_pkt_buf) { git_pkt *pkt; - const char *line = data_pkt->data, *line_end; - size_t line_len = data_pkt->len; + const char *line_end; int error; - while (line_len > 0) { - error = git_pkt_parse_line(&pkt, line, &line_end, line_len); + while (data_pkt_buf->size > 0) { + error = git_pkt_parse_line(&pkt, data_pkt_buf->ptr, &line_end, data_pkt_buf->size); if (error < 0) return error; /* Advance in the buffer */ - line_len -= (line_end - line); - line = line_end; + git_buf_consume(data_pkt_buf, line_end); error = add_push_report_pkt(push, pkt); @@ -755,6 +753,8 @@ static int parse_report(transport_smart *transport, git_push *push) const char *line_end = NULL; gitno_buffer *buf = &transport->buffer; int error, recvd; + git_buf data_pkt_buf = GIT_BUF_INIT; + git_pkt_data *data_pkt; for (;;) { if (buf->offset > 0) @@ -763,16 +763,21 @@ static int parse_report(transport_smart *transport, git_push *push) else error = GIT_EBUFS; - if (error < 0 && error != GIT_EBUFS) - return -1; + if (error < 0 && error != GIT_EBUFS) { + error = -1; + goto done; + } if (error == GIT_EBUFS) { - if ((recvd = gitno_recv(buf)) < 0) - return recvd; + if ((recvd = gitno_recv(buf)) < 0) { + error = recvd; + goto done; + } if (recvd == 0) { giterr_set(GITERR_NET, "early EOF"); - return GIT_EEOF; + error = GIT_EEOF; + goto done; } continue; } @@ -783,8 +788,14 @@ static int parse_report(transport_smart *transport, git_push *push) switch (pkt->type) { case GIT_PKT_DATA: - /* This is a sideband packet which contains other packets */ - error = add_push_report_sideband_pkt(push, (git_pkt_data *)pkt); + /* This is a sideband packet which contains other packets + * Buffer the data in case the inner packet is split + * across multiple sideband packets */ + data_pkt = (git_pkt_data *)pkt; + git_buf_put(&data_pkt_buf, data_pkt->data, data_pkt->len); + error = add_push_report_sideband_pkt(push, &data_pkt_buf); + if (error == GIT_EBUFS) + error = 0; break; case GIT_PKT_ERR: giterr_set(GITERR_NET, "report-status: Error reported: %s", @@ -805,12 +816,24 @@ static int parse_report(transport_smart *transport, git_push *push) git_pkt_free(pkt); /* add_push_report_pkt returns GIT_ITEROVER when it receives a flush */ - if (error == GIT_ITEROVER) - return 0; + if (error == GIT_ITEROVER) { + error = 0; + if (data_pkt_buf.size > 0) { + /* If there was data remaining in the pack data buffer, + * then the server sent a partial pkt-line */ + giterr_set(GITERR_NET, "Incomplete pack data pkt-line"); + error = GIT_ERROR; + } + goto done; + } - if (error < 0) - return error; + if (error < 0) { + goto done; + } } +done: + git_buf_free(&data_pkt_buf); + return error; } static int add_ref_from_push_spec(git_vector *refs, push_spec *push_spec) From cdded6309ae94b66381418b372afc96bd7fab09e Mon Sep 17 00:00:00 2001 From: "P.S.V.R" Date: Wed, 13 Jan 2016 11:07:14 +0800 Subject: [PATCH 10/58] Remove duplicated calls to git_mwindow_close --- src/indexer.c | 1 - src/pack.c | 3 --- 2 files changed, 4 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 6e9af06a5..6266bc888 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -777,7 +777,6 @@ static int fix_thin_pack(git_indexer *idx, git_transfer_progress *stats) curpos = delta->delta_off; error = git_packfile_unpack_header(&size, &type, &idx->pack->mwf, &w, &curpos); - git_mwindow_close(&w); if (error < 0) return error; diff --git a/src/pack.c b/src/pack.c index 52c652178..1028b35b3 100644 --- a/src/pack.c +++ b/src/pack.c @@ -494,7 +494,6 @@ int git_packfile_resolve_header( int error; error = git_packfile_unpack_header(&size, &type, &p->mwf, &w_curs, &curpos); - git_mwindow_close(&w_curs); if (error < 0) return error; @@ -517,7 +516,6 @@ int git_packfile_resolve_header( while (type == GIT_OBJ_OFS_DELTA || type == GIT_OBJ_REF_DELTA) { curpos = base_offset; error = git_packfile_unpack_header(&size, &type, &p->mwf, &w_curs, &curpos); - git_mwindow_close(&w_curs); if (error < 0) return error; if (type != GIT_OBJ_OFS_DELTA && type != GIT_OBJ_REF_DELTA) @@ -585,7 +583,6 @@ static int pack_dependency_chain(git_dependency_chain *chain_out, elem->base_key = obj_offset; error = git_packfile_unpack_header(&size, &type, &p->mwf, &w_curs, &curpos); - git_mwindow_close(&w_curs); if (error < 0) goto on_error; From eb09ead246402da8d95f236734b037a2cc74456f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Fri, 4 Mar 2016 01:18:30 -0500 Subject: [PATCH 11/58] odb: improved not found error messages When looking up an abbreviated oid, show the actual (abbreviated) oid the caller passed instead of a full (but ambiguously truncated) oid. --- src/odb.c | 17 ++++++++++------- src/odb.h | 3 ++- src/odb_loose.c | 20 ++++++++++++-------- src/odb_pack.c | 8 +++++--- src/pack.c | 10 +++++----- 5 files changed, 34 insertions(+), 24 deletions(-) diff --git a/src/odb.c b/src/odb.c index 1c877c9fc..cb0f70623 100644 --- a/src/odb.c +++ b/src/odb.c @@ -725,7 +725,8 @@ int git_odb_exists_prefix( git_oid_cpy(out, short_id); return 0; } else { - return git_odb__error_notfound("no match for id prefix", short_id); + return git_odb__error_notfound( + "no match for id prefix", short_id, len); } } @@ -740,7 +741,7 @@ int git_odb_exists_prefix( error = odb_exists_prefix_1(out, db, &key, len, true); if (error == GIT_ENOTFOUND) - return git_odb__error_notfound("no match for id prefix", &key); + return git_odb__error_notfound("no match for id prefix", &key, len); return error; } @@ -881,7 +882,7 @@ int git_odb_read(git_odb_object **out, git_odb *db, const git_oid *id) error = odb_read_1(out, db, id, true); if (error == GIT_ENOTFOUND) - return git_odb__error_notfound("no match for id", id); + return git_odb__error_notfound("no match for id", id, GIT_OID_HEXSZ); return error; } @@ -967,7 +968,7 @@ int git_odb_read_prefix( error = read_prefix_1(out, db, &key, len, true); if (error == GIT_ENOTFOUND) - return git_odb__error_notfound("no match for prefix", &key); + return git_odb__error_notfound("no match for prefix", &key, len); return error; } @@ -1223,12 +1224,14 @@ int git_odb_refresh(struct git_odb *db) return 0; } -int git_odb__error_notfound(const char *message, const git_oid *oid) +int git_odb__error_notfound( + const char *message, const git_oid *oid, size_t oid_len) { if (oid != NULL) { char oid_str[GIT_OID_HEXSZ + 1]; - git_oid_tostr(oid_str, sizeof(oid_str), oid); - giterr_set(GITERR_ODB, "Object not found - %s (%s)", message, oid_str); + git_oid_tostr(oid_str, oid_len, oid); + giterr_set(GITERR_ODB, "Object not found - %s (%.*s)", + message, oid_len, oid_str); } else giterr_set(GITERR_ODB, "Object not found - %s", message); diff --git a/src/odb.h b/src/odb.h index 281bd3a4d..31a9fd1b9 100644 --- a/src/odb.h +++ b/src/odb.h @@ -82,7 +82,8 @@ int git_odb__hashlink(git_oid *out, const char *path); /* * Generate a GIT_ENOTFOUND error for the ODB. */ -int git_odb__error_notfound(const char *message, const git_oid *oid); +int git_odb__error_notfound( + const char *message, const git_oid *oid, size_t oid_len); /* * Generate a GIT_EAMBIGUOUS error for the ODB. diff --git a/src/odb_loose.c b/src/odb_loose.c index 730c4b1e1..9d9bffd21 100644 --- a/src/odb_loose.c +++ b/src/odb_loose.c @@ -547,7 +547,8 @@ static int locate_object_short_oid( /* Check that directory exists */ if (git_path_isdir(object_location->ptr) == false) - return git_odb__error_notfound("no matching loose object for prefix", short_oid); + return git_odb__error_notfound("no matching loose object for prefix", + short_oid, len); state.dir_len = git_buf_len(object_location); state.short_oid_len = len; @@ -560,7 +561,8 @@ static int locate_object_short_oid( return error; if (!state.found) - return git_odb__error_notfound("no matching loose object for prefix", short_oid); + return git_odb__error_notfound("no matching loose object for prefix", + short_oid, len); if (state.found > 1) return git_odb__error_ambiguous("multiple matches in loose objects"); @@ -613,9 +615,10 @@ static int loose_backend__read_header(size_t *len_p, git_otype *type_p, git_odb_ raw.len = 0; raw.type = GIT_OBJ_BAD; - if (locate_object(&object_path, (loose_backend *)backend, oid) < 0) - error = git_odb__error_notfound("no matching loose object", oid); - else if ((error = read_header_loose(&raw, &object_path)) == 0) { + if (locate_object(&object_path, (loose_backend *)backend, oid) < 0) { + error = git_odb__error_notfound("no matching loose object", + oid, GIT_OID_HEXSZ); + } else if ((error = read_header_loose(&raw, &object_path)) == 0) { *len_p = raw.len; *type_p = raw.type; } @@ -633,9 +636,10 @@ static int loose_backend__read(void **buffer_p, size_t *len_p, git_otype *type_p assert(backend && oid); - if (locate_object(&object_path, (loose_backend *)backend, oid) < 0) - error = git_odb__error_notfound("no matching loose object", oid); - else if ((error = read_loose(&raw, &object_path)) == 0) { + if (locate_object(&object_path, (loose_backend *)backend, oid) < 0) { + error = git_odb__error_notfound("no matching loose object", + oid, GIT_OID_HEXSZ); + } else if ((error = read_loose(&raw, &object_path)) == 0) { *buffer_p = raw.data; *len_p = raw.len; *type_p = raw.type; diff --git a/src/odb_pack.c b/src/odb_pack.c index 77d2c75b9..5a57864ad 100644 --- a/src/odb_pack.c +++ b/src/odb_pack.c @@ -264,7 +264,8 @@ static int pack_entry_find(struct git_pack_entry *e, struct pack_backend *backen if (!pack_entry_find_inner(e, backend, oid, last_found)) return 0; - return git_odb__error_notfound("failed to find pack entry", oid); + return git_odb__error_notfound( + "failed to find pack entry", oid, GIT_OID_HEXSZ); } static int pack_entry_find_prefix( @@ -309,7 +310,8 @@ static int pack_entry_find_prefix( } if (!found) - return git_odb__error_notfound("no matching pack entry for prefix", short_oid); + return git_odb__error_notfound("no matching pack entry for prefix", + short_oid, len); else return 0; } @@ -333,7 +335,7 @@ static int pack_backend__refresh(git_odb_backend *backend_) return 0; if (p_stat(backend->pack_folder, &st) < 0 || !S_ISDIR(st.st_mode)) - return git_odb__error_notfound("failed to refresh packfiles", NULL); + return git_odb__error_notfound("failed to refresh packfiles", NULL, 0); git_buf_sets(&path, backend->pack_folder); diff --git a/src/pack.c b/src/pack.c index 1028b35b3..e7003e66d 100644 --- a/src/pack.c +++ b/src/pack.c @@ -1015,7 +1015,7 @@ static int packfile_open(struct git_pack_file *p) unsigned char *idx_sha1; if (p->index_version == -1 && pack_index_open(p) < 0) - return git_odb__error_notfound("failed to open packfile", NULL); + return git_odb__error_notfound("failed to open packfile", NULL, 0); /* if mwf opened by another thread, return now */ if (git_mutex_lock(&p->lock) < 0) @@ -1096,7 +1096,7 @@ int git_packfile__name(char **out, const char *path) path_len = strlen(path); if (path_len < strlen(".idx")) - return git_odb__error_notfound("invalid packfile path", NULL); + return git_odb__error_notfound("invalid packfile path", NULL, 0); if (git_buf_printf(&buf, "%.*s.pack", (int)(path_len - strlen(".idx")), path) < 0) return -1; @@ -1114,7 +1114,7 @@ int git_packfile_alloc(struct git_pack_file **pack_out, const char *path) *pack_out = NULL; if (path_len < strlen(".idx")) - return git_odb__error_notfound("invalid packfile path", NULL); + return git_odb__error_notfound("invalid packfile path", NULL, 0); GITERR_CHECK_ALLOC_ADD(&alloc_len, sizeof(*p), path_len); GITERR_CHECK_ALLOC_ADD(&alloc_len, alloc_len, 2); @@ -1140,7 +1140,7 @@ int git_packfile_alloc(struct git_pack_file **pack_out, const char *path) if (p_stat(p->pack_name, &st) < 0 || !S_ISREG(st.st_mode)) { git__free(p); - return git_odb__error_notfound("packfile not found", NULL); + return git_odb__error_notfound("packfile not found", NULL, 0); } /* ok, it looks sane as far as we can check without @@ -1341,7 +1341,7 @@ static int pack_entry_find_offset( } if (!found) - return git_odb__error_notfound("failed to find offset for pack entry", short_oid); + return git_odb__error_notfound("failed to find offset for pack entry", short_oid, len); if (found > 1) return git_odb__error_ambiguous("found multiple offsets for pack entry"); From 9ee498e8006b02975c48d850a138ddb50af43717 Mon Sep 17 00:00:00 2001 From: Chris Hescock Date: Tue, 8 Mar 2016 10:16:37 -0500 Subject: [PATCH 12/58] Only buffer if necessary. --- src/transports/smart_protocol.c | 57 ++++++++++++++++++++++----------- 1 file changed, 39 insertions(+), 18 deletions(-) diff --git a/src/transports/smart_protocol.c b/src/transports/smart_protocol.c index 7d381b4cb..02e1ecf74 100644 --- a/src/transports/smart_protocol.c +++ b/src/transports/smart_protocol.c @@ -721,30 +721,58 @@ static int add_push_report_pkt(git_push *push, git_pkt *pkt) return 0; } -static int add_push_report_sideband_pkt(git_push *push, git_buf *data_pkt_buf) +static int add_push_report_sideband_pkt(git_push *push, git_pkt_data *data_pkt, git_buf *data_pkt_buf) { git_pkt *pkt; - const char *line_end; + const char *line, *line_end; + size_t line_len; int error; + int reading_from_buf = data_pkt_buf->size > 0; - while (data_pkt_buf->size > 0) { - error = git_pkt_parse_line(&pkt, data_pkt_buf->ptr, &line_end, data_pkt_buf->size); + if (reading_from_buf) { + /* We had an existing partial packet, so add the new + * packet to the buffer and parse the whole thing */ + git_buf_put(data_pkt_buf, data_pkt->data, data_pkt->len); + line = data_pkt_buf->ptr; + line_len = data_pkt_buf->size; + } + else { + line = data_pkt->data; + line_len = data_pkt->len; + } - if (error < 0) - return error; + while (line_len > 0) { + error = git_pkt_parse_line(&pkt, line, &line_end, line_len); + + if (error == GIT_EBUFS) { + /* Buffer the data when the inner packet is split + * across multiple sideband packets */ + if (!reading_from_buf) + git_buf_put(data_pkt_buf, line, line_len); + error = 0; + goto done; + } + else if (error < 0) + goto done; /* Advance in the buffer */ - git_buf_consume(data_pkt_buf, line_end); + line_len -= (line_end - line); + line = line_end; error = add_push_report_pkt(push, pkt); git_pkt_free(pkt); if (error < 0 && error != GIT_ITEROVER) - return error; + goto done; } - return 0; + error = 0; + +done: + if (reading_from_buf) + git_buf_consume(data_pkt_buf, line_end); + return error; } static int parse_report(transport_smart *transport, git_push *push) @@ -754,7 +782,6 @@ static int parse_report(transport_smart *transport, git_push *push) gitno_buffer *buf = &transport->buffer; int error, recvd; git_buf data_pkt_buf = GIT_BUF_INIT; - git_pkt_data *data_pkt; for (;;) { if (buf->offset > 0) @@ -788,14 +815,8 @@ static int parse_report(transport_smart *transport, git_push *push) switch (pkt->type) { case GIT_PKT_DATA: - /* This is a sideband packet which contains other packets - * Buffer the data in case the inner packet is split - * across multiple sideband packets */ - data_pkt = (git_pkt_data *)pkt; - git_buf_put(&data_pkt_buf, data_pkt->data, data_pkt->len); - error = add_push_report_sideband_pkt(push, &data_pkt_buf); - if (error == GIT_EBUFS) - error = 0; + /* This is a sideband packet which contains other packets */ + error = add_push_report_sideband_pkt(push, (git_pkt_data *)pkt, &data_pkt_buf); break; case GIT_PKT_ERR: giterr_set(GITERR_NET, "report-status: Error reported: %s", From 4ebf745f064ac5a4ef2d2370ca386b5cd6b09fd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 9 Mar 2016 11:16:16 +0100 Subject: [PATCH 13/58] mwindow: free unused windows if we fail to mmap The first time may be due to memory fragmentation or just bad luck on a 32-bit system. When we hit the mmap error for the first time, free up the unused windows and try again. --- src/mwindow.c | 14 ++++++++++++-- 1 file changed, 12 insertions(+), 2 deletions(-) diff --git a/src/mwindow.c b/src/mwindow.c index 55c8d894b..d3e9be78b 100644 --- a/src/mwindow.c +++ b/src/mwindow.c @@ -296,8 +296,18 @@ static git_mwindow *new_window( */ if (git_futils_mmap_ro(&w->window_map, fd, w->offset, (size_t)len) < 0) { - git__free(w); - return NULL; + /* + * The first error might be down to memory fragmentation even if + * we're below our soft limits, so free up what we can and try again. + */ + + while (git_mwindow_close_lru(mwf) == 0) + /* nop */; + + if (git_futils_mmap_ro(&w->window_map, fd, w->offset, (size_t)len) < 0) { + git__free(w); + return NULL; + } } ctl->mmap_calls++; From ffb1f41949d3f4441013fddbcbc7dceb63697a96 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 14:24:09 +0100 Subject: [PATCH 14/58] describe: handle error code returned by git_pqueue_insert --- src/describe.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/describe.c b/src/describe.c index 48f04e858..13ddad5be 100644 --- a/src/describe.c +++ b/src/describe.c @@ -582,7 +582,8 @@ static int describe( best = (struct possible_tag *)git_vector_get(&all_matches, 0); if (gave_up_on) { - git_pqueue_insert(&list, gave_up_on); + if ((error = git_pqueue_insert(&list, gave_up_on)) < 0) + goto cleanup; seen_commits--; } if ((error = finish_depth_computation( From e39ad747f70c9ba166e4db9f41fc222aee6cd3be Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 14:40:17 +0100 Subject: [PATCH 15/58] config_file: handle missing quotation marks in section header When parsing a section header we expect something along the format of '[section "subsection"]'. When a section is mal-formated and is entirely missing its quotation marks we catch this case by observing that `strchr(line, '"') - strrchr(line, '"') = NULL - NULL = 0` and error out. Unfortunately, the error message is misleading though, as we state that we are missing the closing quotation mark while we in fact miss both quotation marks. Improve the error message by explicitly checking if the first quotation mark could be found and, if not, stating that quotation marks are completely missing. --- src/config_file.c | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/config_file.c b/src/config_file.c index 5f5e309e0..65971b930 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -1032,6 +1032,11 @@ static int parse_section_header_ext(struct reader *reader, const char *line, con */ first_quote = strchr(line, '"'); + if (first_quote == NULL) { + set_parse_error(reader, 0, "Missing quotation marks in section header"); + return -1; + } + last_quote = strrchr(line, '"'); quoted_len = last_quote - first_quote; From 0370dae1adea43f250b66abb0d48f9cc5987c15a Mon Sep 17 00:00:00 2001 From: Piet Brauer Date: Thu, 25 Feb 2016 18:15:02 +0800 Subject: [PATCH 16/58] Check for __CLANG_INTTYPES_H This fixes an issue in Xcode 7.3 in objective-git where we get the error "Include of non-modular header file in module". Not importing this header again fixes the issue. --- include/git2/common.h | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/include/git2/common.h b/include/git2/common.h index 4f43185f8..c1efee320 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -24,7 +24,8 @@ GIT_BEGIN_DECL # include "inttypes.h" GIT_END_DECL -#else +/** This check is needed for importing this file in an iOS/OS X framework throws an error in Xcode otherwise.*/ +#elif !defined(__CLANG_INTTYPES_H) # include #endif From faf823dcca8276cbb7ea98e8a531d8deca414de9 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Fri, 11 Mar 2016 09:58:38 +0100 Subject: [PATCH 17/58] tests: transport: fix memory leaks with registering transports --- tests/transport/register.c | 35 +++++++++++++++++------------------ 1 file changed, 17 insertions(+), 18 deletions(-) diff --git a/tests/transport/register.c b/tests/transport/register.c index 67a2efd99..97aae6b20 100644 --- a/tests/transport/register.c +++ b/tests/transport/register.c @@ -40,20 +40,23 @@ void test_transport_register__custom_transport_error_remove_non_existing(void) void test_transport_register__custom_transport_ssh(void) { + const char *urls[] = { + "ssh://somehost:somepath", + "ssh+git://somehost:somepath", + "git+ssh://somehost:somepath", + "git@somehost:somepath", + }; git_transport *transport; + unsigned i; + for (i = 0; i < ARRAY_SIZE(urls); i++) { #ifndef GIT_SSH - cl_git_fail_with(git_transport_new(&transport, NULL, "ssh://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, urls[i]), -1); #else - cl_git_pass(git_transport_new(&transport, NULL, "ssh://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); - transport->free(transport); + cl_git_pass(git_transport_new(&transport, NULL, urls[i])); + transport->free(transport); #endif + } cl_git_pass(git_transport_register("ssh", dummy_transport, NULL)); @@ -63,16 +66,12 @@ void test_transport_register__custom_transport_ssh(void) cl_git_pass(git_transport_unregister("ssh")); + for (i = 0; i < ARRAY_SIZE(urls); i++) { #ifndef GIT_SSH - cl_git_fail_with(git_transport_new(&transport, NULL, "ssh://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath"), -1); - cl_git_fail_with(git_transport_new(&transport, NULL, "git@somehost:somepath"), -1); + cl_git_fail_with(git_transport_new(&transport, NULL, urls[i]), -1); #else - cl_git_pass(git_transport_new(&transport, NULL, "ssh://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "ssh+git://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "git+ssh://somehost:somepath")); - cl_git_pass(git_transport_new(&transport, NULL, "git@somehost:somepath")); - transport->free(transport); + cl_git_pass(git_transport_new(&transport, NULL, urls[i])); + transport->free(transport); #endif + } } From fa4b93a61cf0a28bf8359bc32e96a0ea5020b6b6 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2016 15:57:23 -0400 Subject: [PATCH 18/58] backport git_oid__cpy_prefix --- src/oid.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/src/oid.h b/src/oid.h index aa1f0bfdc..922a2a347 100644 --- a/src/oid.h +++ b/src/oid.h @@ -44,4 +44,13 @@ GIT_INLINE(int) git_oid__cmp(const git_oid *a, const git_oid *b) return git_oid__hashcmp(a->id, b->id); } +GIT_INLINE(void) git_oid__cpy_prefix( + git_oid *out, const git_oid *id, size_t len) +{ + memcpy(&out->id, id->id, (len + 1) / 2); + + if (len & 1) + out->id[len / 2] &= 0xF0; +} + #endif From d0780b8133f40c2e54670eacc6943ff2da49cf84 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 15:35:45 +0100 Subject: [PATCH 19/58] object: avoid call of memset with ouf of bounds pointer When computing a short OID we do this by first copying the leading parts into the new OID structure and then setting the trailing part to zero. In the case of the desired length being `GIT_OID_HEXSZ - 1` we will call `memset` with an out of bounds pointer and a length of 0. While this seems to cause no problems for common platforms the C89 standard does not explicitly state that calling `memset` with an out of bounds pointer and length of 0 is valid. Fix the potential issue by using the newly introduced `git_oid__cpy_prefix` function. --- src/object.c | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/src/object.c b/src/object.c index ebf77fb47..1d45f9f1b 100644 --- a/src/object.c +++ b/src/object.c @@ -12,6 +12,7 @@ #include "commit.h" #include "tree.h" #include "blob.h" +#include "oid.h" #include "tag.h" bool git_object__strict_input_validation = true; @@ -166,13 +167,9 @@ int git_object_lookup_prefix( error = git_odb_read(&odb_obj, odb, id); } } else { - git_oid short_oid; + git_oid short_oid = {{ 0 }}; - /* We copy the first len*4 bits from id and fill the remaining with 0s */ - memcpy(short_oid.id, id->id, (len + 1) / 2); - if (len % 2) - short_oid.id[len / 2] &= 0xF0; - memset(short_oid.id + (len + 1) / 2, 0, (GIT_OID_HEXSZ - len) / 2); + git_oid__cpy_prefix(&short_oid, id, len); /* If len < GIT_OID_HEXSZ (a strict short oid was given), we have * 2 options : From e114bbac892566102e111e34dc6f44d4920bb655 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 16:00:49 +0100 Subject: [PATCH 20/58] index: assert required OID are non-NULL --- src/index.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/index.c b/src/index.c index b97f8091d..62aacf959 100644 --- a/src/index.c +++ b/src/index.c @@ -963,14 +963,20 @@ static int index_entry_reuc_init(git_index_reuc_entry **reuc_out, *reuc_out = reuc = reuc_entry_alloc(path); GITERR_CHECK_ALLOC(reuc); - if ((reuc->mode[0] = ancestor_mode) > 0) + if ((reuc->mode[0] = ancestor_mode) > 0) { + assert(ancestor_oid); git_oid_cpy(&reuc->oid[0], ancestor_oid); + } - if ((reuc->mode[1] = our_mode) > 0) + if ((reuc->mode[1] = our_mode) > 0) { + assert(our_oid); git_oid_cpy(&reuc->oid[1], our_oid); + } - if ((reuc->mode[2] = their_mode) > 0) + if ((reuc->mode[2] = their_mode) > 0) { + assert(their_oid); git_oid_cpy(&reuc->oid[2], their_oid); + } return 0; } From 1a16e8b0577ccdba164f0180365193d75a10b58c Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 17:55:40 +0100 Subject: [PATCH 21/58] pack-objects: fix memory leak on overflow --- src/pack-objects.c | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/pack-objects.c b/src/pack-objects.c index 46fe8f3db..11e13f7d4 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -848,8 +848,10 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, git_packbuilder__cache_unlock(pb); - if (overflow) + if (overflow) { + git__free(delta_buf); return -1; + } trg_object->delta_data = git__realloc(delta_buf, delta_size); GITERR_CHECK_ALLOC(trg_object->delta_data); From d96c06385283e44eb843d0e371f92ca8d4dd4bbb Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Tue, 1 Mar 2016 19:11:33 +0100 Subject: [PATCH 22/58] submodule: avoid passing NULL pointers to strncmp MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit In C89 it is undefined behavior to pass `NULL` pointers to `strncmp` and later on in C99 it has been explicitly stated that functions with an argument declared as `size_t nmemb` specifying the array length shall always have valid parameters, no matter if `nmemb` is 0 or not (see ISO 9899 ยง7.21.1.2). The function `str_equal_no_trailing_slash` always passes its parameters to `strncmp` if their lengths match. This means if one parameter is `NULL` and the other one either `NULL` or a string with length 0 we will pass the pointers to `strncmp` and cause undefined behavior. Fix this by explicitly handling the case when both lengths are 0. --- src/submodule.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/submodule.c b/src/submodule.c index 38db41529..3f39b9ef0 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -80,7 +80,8 @@ static kh_inline int str_equal_no_trailing_slash(const char *a, const char *b) if (blen > 0 && b[blen - 1] == '/') blen--; - return (alen == blen && strncmp(a, b, alen) == 0); + return (alen == 0 && blen == 0) || + (alen == blen && strncmp(a, b, alen) == 0); } __KHASH_IMPL( From 851c51abdf8272e0d1bfdf3597f7da3ea6450373 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 10:40:47 +0100 Subject: [PATCH 23/58] diff_tform: fix potential NULL pointer access When the user passes in a diff which has no repository associated we may call `git_config__get_int_force` with a NULL-pointer configuration. Even though `git_config__get_int_force` is designed to swallow errors, it is not intended to be called with a NULL pointer configuration. Fix the issue by only calling `git_config__get_int_force` only when configuration could be retrieved from the repository. --- src/diff_tform.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/src/diff_tform.c b/src/diff_tform.c index 8577f06b8..6a6a62811 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -261,7 +261,7 @@ static int normalize_find_opts( if (!given || (given->flags & GIT_DIFF_FIND_ALL) == GIT_DIFF_FIND_BY_CONFIG) { - if (diff->repo) { + if (cfg) { char *rule = git_config__get_string_force(cfg, "diff.renames", "true"); int boolval; @@ -318,8 +318,10 @@ static int normalize_find_opts( #undef USE_DEFAULT if (!opts->rename_limit) { - opts->rename_limit = git_config__get_int_force( - cfg, "diff.renamelimit", DEFAULT_RENAME_LIMIT); + if (cfg) { + opts->rename_limit = git_config__get_int_force( + cfg, "diff.renamelimit", DEFAULT_RENAME_LIMIT); + } if (opts->rename_limit <= 0) opts->rename_limit = DEFAULT_RENAME_LIMIT; From 0b357e2eed7f8814706f91c6d92090624572a9ea Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Wed, 2 Mar 2016 01:50:34 +0100 Subject: [PATCH 24/58] coverity: report errors when uploading tarball Curl by default does not report errors by setting the error code. As the upload can fail through several conditions (e.g. the rate limit, leading to unauthorized access) we should indicate this information in Travis CI. To improve upon the behavior, use `--write-out=%{http_code}` to write out the HTTP code in addition to the received body and return an error if the code does not equal 201. --- script/coverity.sh | 18 ++++++++++++++++-- 1 file changed, 16 insertions(+), 2 deletions(-) diff --git a/script/coverity.sh b/script/coverity.sh index 8c826892f..7fe9eb4c7 100755 --- a/script/coverity.sh +++ b/script/coverity.sh @@ -49,10 +49,24 @@ COVERITY_UNSUPPORTED=1 \ # Upload results tar czf libgit2.tgz cov-int SHA=$(git rev-parse --short HEAD) -curl \ + +HTML="$(curl \ + --silent \ + --write-out "\n%{http_code}" \ --form token="$COVERITY_TOKEN" \ --form email=bs@github.com \ --form file=@libgit2.tgz \ --form version="$SHA" \ --form description="Travis build" \ - https://scan.coverity.com/builds?project=libgit2 + https://scan.coverity.com/builds?project=libgit2)" +# Body is everything up to the last line +BODY="$(echo "$HTML" | head -n-1)" +# Status code is the last line +STATUS_CODE="$(echo "$HTML" | tail -n1)" + +echo "${BODY}" + +if [ "${STATUS_CODE}" != "201" ]; then + echo "Received error code ${STATUS_CODE} from Coverity" + exit 1 +fi From 8d3ee96ada7d9441be1976227201775a69e6a2d5 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 16:11:51 +0100 Subject: [PATCH 25/58] refdb_fs: fail if refcache returns NULL pointer We usually check entries returned by `git_sortedcache_entry` for NULL pointers. As we have a write lock in `packed_write`, though, it really should not happen that the function returns NULL. Assert that ref is not NULL to silence a Coverity warning. --- src/refdb_fs.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/refdb_fs.c b/src/refdb_fs.c index f6ed7201a..f978038e6 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -962,6 +962,7 @@ static int packed_write(refdb_fs_backend *backend) for (i = 0; i < git_sortedcache_entrycount(refcache); ++i) { struct packref *ref = git_sortedcache_entry(refcache, i); + assert(ref); if (packed_find_peel(backend, ref) < 0) goto fail; From dd78d7d15b9566e369b9bc48c72a48d77e0c34eb Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 16:33:49 +0100 Subject: [PATCH 26/58] blame_git: handle error returned by `git_commit_parent` --- src/blame_git.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/blame_git.c b/src/blame_git.c index b8b568285..700207edb 100644 --- a/src/blame_git.c +++ b/src/blame_git.c @@ -525,7 +525,8 @@ static int pass_blame(git_blame *blame, git_blame__origin *origin, uint32_t opt) if (sg_origin[i]) continue; - git_commit_parent(&p, origin->commit, i); + if ((error = git_commit_parent(&p, origin->commit, i)) < 0) + goto finish; porigin = find_origin(blame, p, origin); if (!porigin) From f17ed63759d528261f915819a6249c9b77538c55 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 16:42:55 +0100 Subject: [PATCH 27/58] blame: handle error when resoling HEAD in normalize_options When normalizing options we try to look up HEAD's OID. While this action may fail in malformed repositories we never check the return value of the function. Fix the issue by converting `normalize_options` to actually return an error and handle the error in `git_blame_file`. --- src/blame.c | 11 ++++++++--- 1 file changed, 8 insertions(+), 3 deletions(-) diff --git a/src/blame.c b/src/blame.c index 2daf91591..2c8584ba5 100644 --- a/src/blame.c +++ b/src/blame.c @@ -178,7 +178,7 @@ const git_blame_hunk *git_blame_get_hunk_byline(git_blame *blame, size_t lineno) return NULL; } -static void normalize_options( +static int normalize_options( git_blame_options *out, const git_blame_options *in, git_repository *repo) @@ -190,7 +190,9 @@ static void normalize_options( /* No newest_commit => HEAD */ if (git_oid_iszero(&out->newest_commit)) { - git_reference_name_to_id(&out->newest_commit, repo, "HEAD"); + if (git_reference_name_to_id(&out->newest_commit, repo, "HEAD") < 0) { + return -1; + } } /* min_line 0 really means 1 */ @@ -204,6 +206,8 @@ static void normalize_options( out->flags |= GIT_BLAME_TRACK_COPIES_SAME_COMMIT_MOVES; if (out->flags & GIT_BLAME_TRACK_COPIES_SAME_COMMIT_MOVES) out->flags |= GIT_BLAME_TRACK_COPIES_SAME_FILE; + + return 0; } static git_blame_hunk *split_hunk_in_vector( @@ -362,7 +366,8 @@ int git_blame_file( git_blame *blame = NULL; assert(out && repo && path); - normalize_options(&normOptions, options, repo); + if ((error = normalize_options(&normOptions, options, repo)) < 0) + goto on_error; blame = git_blame__alloc(repo, normOptions, path); GITERR_CHECK_ALLOC(blame); From 18c4ae70d154ef1b3c38d7950cdd59b8ad08e9c2 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 17:05:30 +0100 Subject: [PATCH 28/58] filebuf: handle write error in `lock_file` When writing to a file with locking not check if writing the locked file actually succeeds. Fix the issue by returning error code and message when writing fails. --- src/filebuf.c | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/filebuf.c b/src/filebuf.c index 17efe872e..101d5082a 100644 --- a/src/filebuf.c +++ b/src/filebuf.c @@ -70,6 +70,7 @@ static int lock_file(git_filebuf *file, int flags, mode_t mode) git_file source; char buffer[FILEIO_BUFSIZE]; ssize_t read_bytes; + int error; source = p_open(file->path_original, O_RDONLY); if (source < 0) { @@ -80,7 +81,8 @@ static int lock_file(git_filebuf *file, int flags, mode_t mode) } while ((read_bytes = p_read(source, buffer, sizeof(buffer))) > 0) { - p_write(file->fd, buffer, read_bytes); + if ((error = p_write(file->fd, buffer, read_bytes)) < 0) + break; if (file->compute_digest) git_hash_update(&file->digest, buffer, read_bytes); } @@ -90,6 +92,9 @@ static int lock_file(git_filebuf *file, int flags, mode_t mode) if (read_bytes < 0) { giterr_set(GITERR_OS, "Failed to read file '%s'", file->path_original); return -1; + } else if (error < 0) { + giterr_set(GITERR_OS, "Failed to write file '%s'", file->path_lock); + return -1; } } From 89e7604c3a55ac5f6e8888e483962a13baad76ae Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 10 Mar 2016 17:21:02 +0100 Subject: [PATCH 29/58] config_cache: check return value of `git_config__lookup_entry` Callers of `git_config__cvar` already handle the case where the function returns an error due to a failed configuration variable lookup, but we are actually swallowing errors when calling `git_config__lookup_entry` inside of the function. Fix this by returning early when `git_config__lookup_entry` returns an error. As we call `git_config__lookup_entry` with `no_errors == false` which leads us to call `get_entry` with `GET_NO_MISSING` we will not return early when the lookup fails due to a missing entry. Like this we are still able to set the default value of the cvar and exit successfully. --- src/config_cache.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/config_cache.c b/src/config_cache.c index c859ec148..dbea871b9 100644 --- a/src/config_cache.c +++ b/src/config_cache.c @@ -86,7 +86,8 @@ int git_config__cvar(int *out, git_config *config, git_cvar_cached cvar) struct map_data *data = &_cvar_maps[(int)cvar]; git_config_entry *entry; - git_config__lookup_entry(&entry, config, data->cvar_name, false); + if ((error = git_config__lookup_entry(&entry, config, data->cvar_name, false)) < 0) + return error; if (!entry) *out = data->default_value; From c1ec732f4634c291fefb88ee0dca8539e2fff524 Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 14 Mar 2016 12:02:00 +0000 Subject: [PATCH 30/58] Setup better defaults for OpenSSL ciphers This ensures that when using OpenSSL a safe default set of ciphers is selected. This is done so that the client communicates securely and we don't accidentally enable unsafe ciphers like RC4, or even worse some old export ciphers. Implements the first part of https://github.com/libgit2/libgit2/issues/3682 --- include/git2/common.h | 6 ++++++ src/global.c | 2 ++ src/global.h | 1 + src/openssl_stream.c | 13 +++++++++++++ src/settings.c | 22 ++++++++++++++++++++++ tests/online/badssl.c | 9 +++++++++ 6 files changed, 53 insertions(+) diff --git a/include/git2/common.h b/include/git2/common.h index c1efee320..0629abb7f 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -149,6 +149,7 @@ typedef enum { GIT_OPT_SET_SSL_CERT_LOCATIONS, GIT_OPT_SET_USER_AGENT, GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, + GIT_OPT_SET_SSL_CIPHERS, } git_libgit2_opt_t; /** @@ -260,6 +261,11 @@ typedef enum { * > example, when this is enabled, the parent(s) and tree inputs * > will be validated when creating a new commit. This defaults * > to disabled. + * * opts(GIT_OPT_SET_SSL_CIPHERS, const char *ciphers) + * + * > Set the SSL ciphers use for HTTPS connections. + * > + * > - `ciphers` is the list of ciphers that are eanbled. * * @param option Option key * @param ... value to set the option diff --git a/src/global.c b/src/global.c index 0bfde1e04..c725b5184 100644 --- a/src/global.c +++ b/src/global.c @@ -27,6 +27,7 @@ static git_global_shutdown_fn git__shutdown_callbacks[MAX_SHUTDOWN_CB]; static git_atomic git__n_shutdown_callbacks; static git_atomic git__n_inits; char *git__user_agent; +char *git__ssl_ciphers; void git__on_shutdown(git_global_shutdown_fn callback) { @@ -83,6 +84,7 @@ static void shutdown_common(void) } git__free(git__user_agent); + git__free(git__ssl_ciphers); #if defined(GIT_MSVC_CRTDBG) git_win32__crtdbg_stacktrace_cleanup(); diff --git a/src/global.h b/src/global.h index 9fdcee573..219951525 100644 --- a/src/global.h +++ b/src/global.h @@ -36,5 +36,6 @@ extern void git__on_shutdown(git_global_shutdown_fn callback); extern void git__free_tls_data(void); extern const char *git_libgit2__user_agent(void); +extern const char *git_libgit2__ssl_ciphers(void); #endif diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 97736b714..a65f5586e 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -34,6 +34,8 @@ SSL_CTX *git__ssl_ctx; +#define GIT_SSL_DEFAULT_CIPHERS "ECDHE-ECDSA-AES128-GCM-SHA256:ECDHE-RSA-AES128-GCM-SHA256:ECDHE-ECDSA-AES256-GCM-SHA384:ECDHE-RSA-AES256-GCM-SHA384:DHE-RSA-AES128-GCM-SHA256:DHE-DSS-AES128-GCM-SHA256:DHE-RSA-AES256-GCM-SHA384:DHE-DSS-AES256-GCM-SHA384:ECDHE-ECDSA-AES128-SHA256:ECDHE-RSA-AES128-SHA256:ECDHE-ECDSA-AES128-SHA:ECDHE-RSA-AES128-SHA:ECDHE-ECDSA-AES256-SHA384:ECDHE-RSA-AES256-SHA384:ECDHE-ECDSA-AES256-SHA:ECDHE-RSA-AES256-SHA:DHE-RSA-AES128-SHA256:DHE-RSA-AES256-SHA256:DHE-RSA-AES128-SHA:DHE-RSA-AES256-SHA:DHE-DSS-AES128-SHA256:DHE-DSS-AES256-SHA256:DHE-DSS-AES128-SHA:DHE-DSS-AES256-SHA:AES128-GCM-SHA256:AES256-GCM-SHA384:AES128-SHA256:AES256-SHA256:AES128-SHA:AES256-SHA" + #ifdef GIT_THREADS static git_mutex *openssl_locks; @@ -85,6 +87,7 @@ int git_openssl_stream_global_init(void) { #ifdef GIT_OPENSSL long ssl_opts = SSL_OP_NO_SSLv2 | SSL_OP_NO_SSLv3; + const char *ciphers = git_libgit2__ssl_ciphers(); /* Older OpenSSL and MacOS OpenSSL doesn't have this */ #ifdef SSL_OP_NO_COMPRESSION @@ -108,6 +111,16 @@ int git_openssl_stream_global_init(void) git__ssl_ctx = NULL; return -1; } + + if (!ciphers) { + ciphers = GIT_SSL_DEFAULT_CIPHERS; + } + + if(!SSL_CTX_set_cipher_list(git__ssl_ctx, ciphers)) { + SSL_CTX_free(git__ssl_ctx); + git__ssl_ctx = NULL; + return -1; + } #endif git__on_shutdown(shutdown_ssl); diff --git a/src/settings.c b/src/settings.c index 88602bad0..c0e503d37 100644 --- a/src/settings.c +++ b/src/settings.c @@ -71,12 +71,18 @@ static int config_level_to_sysdir(int config_level) } extern char *git__user_agent; +extern char *git__ssl_ciphers; const char *git_libgit2__user_agent() { return git__user_agent; } +const char *git_libgit2__ssl_ciphers() +{ + return git__ssl_ciphers; +} + int git_libgit2_opts(int key, ...) { int error = 0; @@ -187,6 +193,22 @@ int git_libgit2_opts(int key, ...) git_object__strict_input_validation = (va_arg(ap, int) != 0); break; + case GIT_OPT_SET_SSL_CIPHERS: +#ifdef GIT_OPENSSL + { + git__free(git__ssl_ciphers); + git__ssl_ciphers = git__strdup(va_arg(ap, const char *)); + if (!git__ssl_ciphers) { + giterr_set_oom(); + error = -1; + } + } +#else + giterr_set(GITERR_NET, "Cannot set custom ciphers: OpenSSL is not enabled"); + error = -1; +#endif + break; + default: giterr_set(GITERR_INVALID, "invalid option key"); error = -1; diff --git a/tests/online/badssl.c b/tests/online/badssl.c index 12badbda3..141f22f92 100644 --- a/tests/online/badssl.c +++ b/tests/online/badssl.c @@ -36,3 +36,12 @@ void test_online_badssl__self_signed(void) cl_git_fail_with(GIT_ECERTIFICATE, git_clone(&g_repo, "https://self-signed.badssl.com/fake.git", "./fake", NULL)); } + +void test_online_badssl__old_cipher(void) +{ + if (!g_has_ssl) + cl_skip(); + + cl_git_fail_with(GIT_ERROR, + git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", NULL)); +} From 4e91020c85e1f7cdda44acded4f88c0d4adb538d Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 14 Mar 2016 12:41:12 +0000 Subject: [PATCH 31/58] Start error string with lower case character --- src/settings.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/settings.c b/src/settings.c index c0e503d37..0da19ea03 100644 --- a/src/settings.c +++ b/src/settings.c @@ -175,7 +175,7 @@ int git_libgit2_opts(int key, ...) } } #else - giterr_set(GITERR_NET, "Cannot set certificate locations: OpenSSL is not enabled"); + giterr_set(GITERR_NET, "cannot set certificate locations: OpenSSL is not enabled"); error = -1; #endif break; @@ -204,7 +204,7 @@ int git_libgit2_opts(int key, ...) } } #else - giterr_set(GITERR_NET, "Cannot set custom ciphers: OpenSSL is not enabled"); + giterr_set(GITERR_NET, "cannot set custom ciphers: OpenSSL is not enabled"); error = -1; #endif break; From cdde081b35ea89a91375b2af8e0e67247a505f1a Mon Sep 17 00:00:00 2001 From: Dirkjan Bussink Date: Mon, 14 Mar 2016 12:41:41 +0000 Subject: [PATCH 32/58] Use general cl_git_fail because the error is generic --- tests/online/badssl.c | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tests/online/badssl.c b/tests/online/badssl.c index 141f22f92..66b090df4 100644 --- a/tests/online/badssl.c +++ b/tests/online/badssl.c @@ -42,6 +42,5 @@ void test_online_badssl__old_cipher(void) if (!g_has_ssl) cl_skip(); - cl_git_fail_with(GIT_ERROR, - git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", NULL)); + cl_git_fail(git_clone(&g_repo, "https://rc4.badssl.com/fake.git", "./fake", NULL)); } From f587f38c087834a995c8aa43a1e6b917fe68a46f Mon Sep 17 00:00:00 2001 From: Marc Strapetz Date: Tue, 15 Mar 2016 18:20:32 +0100 Subject: [PATCH 33/58] CMake: do not overwrite but only append to CMAKE_C_FLAGS_DEBUG This is useful to force "smart" IDEs (like CLIon) to use debug flag -g even it may have decided that "-D_DEBUG" (which is already present) is sufficient. --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index f4e56e6cf..b211a2eaf 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -412,7 +412,7 @@ IF (MSVC) # /MTd - Statically link the multithreaded debug version of the CRT # /MDd - Dynamically link the multithreaded debug version of the CRT # /RTC1 - Run time checks - SET(CMAKE_C_FLAGS_DEBUG "/Zi /Od /D_DEBUG /RTC1 ${CRT_FLAG_DEBUG}") + SET(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} /Zi /Od /D_DEBUG /RTC1 ${CRT_FLAG_DEBUG}") # /DNDEBUG - Disables asserts # /MT - Statically link the multithreaded release version of the CRT @@ -464,7 +464,7 @@ ELSE () ENDIF() IF (WIN32 AND NOT CYGWIN) - SET(CMAKE_C_FLAGS_DEBUG "-D_DEBUG") + SET(CMAKE_C_FLAGS_DEBUG "${CMAKE_C_FLAGS_DEBUG} -D_DEBUG") ENDIF () IF (MINGW) # MinGW always does PIC and complains if we tell it to From 9a668abd34f2e2157e2f2c9400614c30fe58af14 Mon Sep 17 00:00:00 2001 From: Marc Strapetz Date: Tue, 15 Mar 2016 18:32:37 +0100 Subject: [PATCH 34/58] Option "LIBGIT2_PREFIX" to set the CMAKE's TARGET_PROPERTIES PREFIX This is especially useful in combination with MinGW to yield the Windows-compliant DLL name "git2.dll" instead of "libgit2.dll" --- CMakeLists.txt | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CMakeLists.txt b/CMakeLists.txt index b211a2eaf..c79b2637c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -608,6 +608,8 @@ IF (SONAME) IF (LIBGIT2_FILENAME) ADD_DEFINITIONS(-DLIBGIT2_FILENAME=\"${LIBGIT2_FILENAME}\") SET_TARGET_PROPERTIES(git2 PROPERTIES OUTPUT_NAME ${LIBGIT2_FILENAME}) + ELSEIF (DEFINED LIBGIT2_PREFIX) + SET_TARGET_PROPERTIES(git2 PROPERTIES PREFIX "${LIBGIT2_PREFIX}") ENDIF() ENDIF() STRING(REPLACE ";" " " LIBGIT2_PC_LIBS "${LIBGIT2_PC_LIBS}") From d8fcafb2ca384a9b5e89ea922c515145fb050e46 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Wed, 16 Mar 2016 19:05:11 +0100 Subject: [PATCH 35/58] Split the page size from the mmap alignment While often similar, these are not the same on Windows. We want to use the page size on Windows for the pools, but for mmap we need to use the allocation granularity as the alignment. On the other platforms these values remain the same. --- src/indexer.c | 8 ++++---- src/posix.c | 7 +++++++ src/posix.h | 1 + src/unix/map.c | 5 +++++ src/win32/map.c | 29 ++++++++++++++++++++++++----- 5 files changed, 41 insertions(+), 9 deletions(-) diff --git a/src/indexer.c b/src/indexer.c index 6266bc888..a3a866989 100644 --- a/src/indexer.c +++ b/src/indexer.c @@ -449,7 +449,7 @@ static void hash_partially(git_indexer *idx, const uint8_t *data, size_t size) static int write_at(git_indexer *idx, const void *data, git_off_t offset, size_t size) { git_file fd = idx->pack->mwf.fd; - size_t page_size; + size_t mmap_alignment; size_t page_offset; git_off_t page_start; unsigned char *map_data; @@ -458,11 +458,11 @@ static int write_at(git_indexer *idx, const void *data, git_off_t offset, size_t assert(data && size); - if ((error = git__page_size(&page_size)) < 0) + if ((error = git__mmap_alignment(&mmap_alignment)) < 0) return error; - /* the offset needs to be at the beginning of the a page boundary */ - page_offset = offset % page_size; + /* the offset needs to be at the mmap boundary for the platform */ + page_offset = offset % mmap_alignment; page_start = offset - page_offset; if ((error = p_mmap(&map, page_offset + size, GIT_PROT_WRITE, GIT_MAP_SHARED, fd, page_start)) < 0) diff --git a/src/posix.c b/src/posix.c index c7201ba14..b3f1a1cd3 100644 --- a/src/posix.c +++ b/src/posix.c @@ -224,6 +224,13 @@ int git__page_size(size_t *page_size) return 0; } +int git__mmap_alignment(size_t *alignment) +{ + /* dummy; here we don't need any alignment anyway */ + *alignment = 4096; + return 0; +} + int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset) { diff --git a/src/posix.h b/src/posix.h index 8785a4c99..f204751cf 100644 --- a/src/posix.h +++ b/src/posix.h @@ -109,6 +109,7 @@ extern int p_getcwd(char *buffer_out, size_t size); extern int p_rename(const char *from, const char *to); extern int git__page_size(size_t *page_size); +extern int git__mmap_alignment(size_t *page_size); /** * Platform-dependent methods diff --git a/src/unix/map.c b/src/unix/map.c index 72abb3418..c55ad1aa7 100644 --- a/src/unix/map.c +++ b/src/unix/map.c @@ -24,6 +24,11 @@ int git__page_size(size_t *page_size) return 0; } +int git__mmap_alignment(size_t *alignment) +{ + return git__page_size(alignment); +} + int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset) { int mprot = PROT_READ; diff --git a/src/win32/map.c b/src/win32/map.c index a99c30f7e..03a3646a6 100644 --- a/src/win32/map.c +++ b/src/win32/map.c @@ -17,22 +17,41 @@ static DWORD get_page_size(void) if (!page_size) { GetSystemInfo(&sys); - page_size = sys.dwAllocationGranularity; + page_size = sys.dwPageSize; } return page_size; } +static DWORD get_allocation_granularity(void) +{ + static DWORD granularity; + SYSTEM_INFO sys; + + if (!granularity) { + GetSystemInfo(&sys); + granularity = sys.dwAllocationGranularity; + } + + return granularity; +} + int git__page_size(size_t *page_size) { *page_size = get_page_size(); return 0; } +int git__mmap_alignment(size_t *page_size) +{ + *page_size = get_allocation_granularity(); + return 0; +} + int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offset) { HANDLE fh = (HANDLE)_get_osfhandle(fd); - DWORD page_size = get_page_size(); + DWORD alignment = get_allocation_granularity(); DWORD fmap_prot = 0; DWORD view_prot = 0; DWORD off_low = 0; @@ -62,12 +81,12 @@ int p_mmap(git_map *out, size_t len, int prot, int flags, int fd, git_off_t offs if (prot & GIT_PROT_READ) view_prot |= FILE_MAP_READ; - page_start = (offset / page_size) * page_size; + page_start = (offset / alignment) * alignment; page_offset = offset - page_start; - if (page_offset != 0) { /* offset must be multiple of page size */ + if (page_offset != 0) { /* offset must be multiple of the allocation granularity */ errno = EINVAL; - giterr_set(GITERR_OS, "Failed to mmap. Offset must be multiple of page size"); + giterr_set(GITERR_OS, "Failed to mmap. Offset must be multiple of allocation granularity"); return -1; } From e97d2d70003f864e7aa24834c7884c8891dc7d8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 17 Mar 2016 10:45:22 +0100 Subject: [PATCH 36/58] commit: fix extraction of single-line signatures The function to extract signatures suffers from a similar bug to the header field finding one by having an unecessary line feed check as a break condition of its loop. Fix that and add a test for this single-line signature situation. --- src/commit.c | 2 +- tests/commit/parse.c | 24 ++++++++++++++++++++++++ 2 files changed, 25 insertions(+), 1 deletion(-) diff --git a/src/commit.c b/src/commit.c index 685c642aa..5ed9c474d 100644 --- a/src/commit.c +++ b/src/commit.c @@ -676,7 +676,7 @@ int git_commit_extract_signature(git_buf *signature, git_buf *signed_data, git_r buf = git_odb_object_data(obj); - while ((h = strchr(buf, '\n')) && h[1] != '\0' && h[1] != '\n') { + while ((h = strchr(buf, '\n')) && h[1] != '\0') { h++; if (git__prefixcmp(buf, field)) { if (git_buf_put(signed_data, buf, h - buf) < 0) diff --git a/tests/commit/parse.c b/tests/commit/parse.c index 838cfb467..297fccc6b 100644 --- a/tests/commit/parse.c +++ b/tests/commit/parse.c @@ -498,6 +498,21 @@ committer Ben Burkert 1358451456 -0800\n\ \n\ a simple commit which works\n"; + const char *oneline_signature = "tree 51832e6397b30309c8bcad9c55fa6ae67778f378\n\ +parent a1b6decaaac768b5e01e1b5dbf5b2cc081bed1eb\n\ +author Some User 1454537944 -0700\n\ +committer Some User 1454537944 -0700\n\ +gpgsig bad\n\ +\n\ +corrupt signature\n"; + + const char *oneline_data = "tree 51832e6397b30309c8bcad9c55fa6ae67778f378\n\ +parent a1b6decaaac768b5e01e1b5dbf5b2cc081bed1eb\n\ +author Some User 1454537944 -0700\n\ +committer Some User 1454537944 -0700\n\ +\n\ +corrupt signature\n"; + cl_git_pass(git_repository_odb__weakptr(&odb, g_repo)); cl_git_pass(git_odb_write(&commit_id, odb, passing_commit_cases[4], strlen(passing_commit_cases[4]), GIT_OBJ_COMMIT)); @@ -523,6 +538,15 @@ a simple commit which works\n"; cl_git_fail_with(GIT_ENOTFOUND, git_commit_extract_signature(&signature, &signed_data, g_repo, &commit_id, NULL)); cl_assert_equal_i(GITERR_OBJECT, giterr_last()->klass); + /* Parse the commit with a single-line signature */ + git_buf_clear(&signature); + git_buf_clear(&signed_data); + cl_git_pass(git_odb_write(&commit_id, odb, oneline_signature, strlen(oneline_signature), GIT_OBJ_COMMIT)); + cl_git_pass(git_commit_extract_signature(&signature, &signed_data, g_repo, &commit_id, NULL)); + cl_assert_equal_s("bad", signature.ptr); + cl_assert_equal_s(oneline_data, signed_data.ptr); + + git_buf_free(&signature); git_buf_free(&signed_data); From a1cf26448a75f20ecb72f6a1648edee717219c59 Mon Sep 17 00:00:00 2001 From: Carlos Martin Nieto Date: Fri, 18 Mar 2016 13:00:27 -0700 Subject: [PATCH 37/58] win32: free thread-local data on thread exit --- src/global.c | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/src/global.c b/src/global.c index c725b5184..adf353d35 100644 --- a/src/global.c +++ b/src/global.c @@ -224,6 +224,20 @@ void git__free_tls_data(void) TlsSetValue(_tls_index, NULL); } +BOOL WINAPI DllMain(HINSTANCE hInstDll, DWORD fdwReason, LPVOID lpvReserved) +{ + /* This is how Windows lets us know our thread is being shut down */ + if (fdwReason == DLL_THREAD_DETACH) { + git__free_tls_data(); + } + + /* + * Windows pays attention to this during library loading. We don't do anything + * so we trivially succeed. + */ + return TRUE; +} + #elif defined(GIT_THREADS) && defined(_POSIX_THREADS) static pthread_key_t _tls_key; From c86a65be4c86508a23f32f2b63fc6042b44f53e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 21 Mar 2016 21:10:26 +0100 Subject: [PATCH 38/58] config: don't special-case multivars that don't exist yet This special-casing ignores that we might have a locked file, so the hashtable does not represent the contents of the file we want to write. This causes multivar writes to overwrite entries instead of add to them when under lock. There is no need for this as the normal code-path will write to the file just fine, so simply get rid of it. --- src/config_file.c | 16 ---------------- tests/config/multivar.c | 2 +- 2 files changed, 1 insertion(+), 17 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index 65971b930..f49596cc0 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -553,30 +553,15 @@ static int config_set_multivar( git_config_backend *cfg, const char *name, const char *regexp, const char *value) { diskfile_backend *b = (diskfile_backend *)cfg; - refcounted_strmap *map; - git_strmap *values; char *key; regex_t preg; int result; - khiter_t pos; assert(regexp); if ((result = git_config__normalize_name(name, &key)) < 0) return result; - map = refcounted_strmap_take(&b->header); - values = b->header.values->values; - - pos = git_strmap_lookup_index(values, key); - if (!git_strmap_valid_index(values, pos)) { - /* If we don't have it, behave like a normal set */ - result = config_set(cfg, name, value); - refcounted_strmap_free(map); - git__free(key); - return result; - } - result = regcomp(&preg, regexp, REG_EXTENDED); if (result != 0) { giterr_set_regex(&preg, result); @@ -591,7 +576,6 @@ static int config_set_multivar( result = config_refresh(cfg); out: - refcounted_strmap_free(map); git__free(key); regfree(&preg); diff --git a/tests/config/multivar.c b/tests/config/multivar.c index 015008992..d1b8c4cda 100644 --- a/tests/config/multivar.c +++ b/tests/config/multivar.c @@ -163,7 +163,7 @@ void test_config_multivar__add_new(void) cl_git_pass(git_config_open_ondisk(&cfg, "config/config11")); - cl_git_pass(git_config_set_multivar(cfg, var, "", "variable")); + cl_git_pass(git_config_set_multivar(cfg, var, "$^", "variable")); n = 0; cl_git_pass(git_config_get_multivar_foreach(cfg, var, NULL, cb, &n)); cl_assert_equal_i(n, 1); From 3ec0f2e37d7bd34cf2dfbf9dd380a67542c1878b Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 31 Mar 2016 11:30:31 +0200 Subject: [PATCH 39/58] xdiff/xprepare: use the XDF_DIFF_ALG() macro to access flag bits Commit 307ab20b3 ("xdiff: PATIENCE/HISTOGRAM are not independent option bits", 19-02-2012) introduced the XDF_DIFF_ALG() macro to access the flag bits used to represent the diff algorithm requested. In addition, code which had used explicit manipulation of the flag bits was changed to use the macros. However, one example of direct manipulation remains. Update this code to use the XDF_DIFF_ALG() macro. This patch was originally written by Ramsay Jones (see commit 5cd6978a9cfef58de061a9525f3678ade479564d in git.git). --- src/xdiff/xprepare.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xdiff/xprepare.c b/src/xdiff/xprepare.c index 63a22c630..ffb70df49 100644 --- a/src/xdiff/xprepare.c +++ b/src/xdiff/xprepare.c @@ -304,7 +304,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; } - if (!(xpp->flags & XDF_HISTOGRAM_DIFF)) + if (XDF_DIFF_ALG((xpp->flags) & XDF_HISTOGRAM_DIFF)) xdl_free_classifier(&cf); return 0; From 56da07cbcb579d679a2e2096b0588aa83e987a71 Mon Sep 17 00:00:00 2001 From: Patrick Steinhardt Date: Thu, 31 Mar 2016 11:32:36 +0200 Subject: [PATCH 40/58] xdiff/xprepare: fix a memory leak The xdl_prepare_env() function may initialise an xdlclassifier_t data structure via xdl_init_classifier(), which allocates memory to several fields, for example 'rchash', 'rcrecs' and 'ncha'. If this function later exits due to the failure of xdl_optimize_ctxs(), then this xdlclassifier_t structure, and the memory allocated to it, is not cleaned up. In order to fix the memory leak, insert a call to xdl_free_classifier() before returning. This patch was originally written by Ramsay Jones (see commit 87f16258367a3b9a62663b11f898a4a6f3c19d31 in git.git). --- src/xdiff/xprepare.c | 1 + 1 file changed, 1 insertion(+) diff --git a/src/xdiff/xprepare.c b/src/xdiff/xprepare.c index ffb70df49..3183d50eb 100644 --- a/src/xdiff/xprepare.c +++ b/src/xdiff/xprepare.c @@ -301,6 +301,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, xdl_free_ctx(&xe->xdf2); xdl_free_ctx(&xe->xdf1); + xdl_free_classifier(&cf); return -1; } From fe1f47776f7fffbb31fd8de067aed9eca25e0b4b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Thu, 31 Mar 2016 11:35:53 +0200 Subject: [PATCH 41/58] Add a no-op size_t typedef for the doc parser Clang's documentation parser, which we use in our documentation system does not report any comments for functions which use size_t as a type. The root cause is buried somewhere in libclang but we can work around it by defining the type ourselves. This typedef makes sure that libclang sees it and that we do not change its size. --- include/git2/common.h | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/include/git2/common.h b/include/git2/common.h index 0629abb7f..d7428d811 100644 --- a/include/git2/common.h +++ b/include/git2/common.h @@ -29,6 +29,14 @@ # include #endif +#ifdef DOCURIUM +/* + * This is so clang's doc parser acknowledges comments on functions + * with size_t parameters. + */ +typedef size_t size_t; +#endif + /** Declare a public function exported for application use. */ #if __GNUC__ >= 4 # define GIT_EXTERN(type) extern \ From ab062a393896496b773c3bfa95f8c9c57657ebed Mon Sep 17 00:00:00 2001 From: Andreas Henriksson Date: Wed, 6 Apr 2016 10:37:30 +0200 Subject: [PATCH 42/58] tests: fix core/stream test when built with openssl off When passing -DUSE_OPENSSL:BOOL=OFF to cmake the testsuite will fail with the following error: core::stream::register_tls [/tmp/libgit2/tests/core/stream.c:40] Function call failed: (error) error -1 - Fix test to assume failure for tls when built without openssl. While at it also fix GIT_WIN32 cpp to check if it's defined or not. --- tests/core/stream.c | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/tests/core/stream.c b/tests/core/stream.c index ace6a05da..0cbf44230 100644 --- a/tests/core/stream.c +++ b/tests/core/stream.c @@ -33,8 +33,12 @@ void test_core_stream__register_tls(void) cl_git_pass(git_stream_register_tls(NULL)); error = git_tls_stream_new(&stream, "localhost", "443"); - /* We don't have arbitrary TLS stream support on Windows */ -#if GIT_WIN32 + /* We don't have arbitrary TLS stream support on Windows + * or when openssl support is disabled (except on OSX + * with Security framework). + */ +#if defined(GIT_WIN32) || \ + (!defined(GIT_SECURE_TRANSPORT) && !defined(GIT_OPENSSL)) cl_git_fail_with(-1, error); #else cl_git_pass(error); From 3e2e8240c02bced3ef919b5da543fd2a7abb9dab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 11 Apr 2016 17:43:07 +0200 Subject: [PATCH 43/58] refs: provide a more general error message for dwim If we cannot dwim the input, set the error message to be explicit about that. Otherwise we leave the error for the last failed lookup, which can be rather unexpected as it mentions a remote when the user thought they were trying to look up a branch. --- src/refs.c | 3 +++ tests/refs/lookup.c | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/src/refs.c b/src/refs.c index a15e31b53..26c80021f 100644 --- a/src/refs.c +++ b/src/refs.c @@ -289,6 +289,9 @@ cleanup: "Could not use '%s' as valid reference name", git_buf_cstr(&name)); } + if (error == GIT_ENOTFOUND) + giterr_set(GITERR_REFERENCE, "no reference found for shorthand '%s'", refname); + git_buf_free(&name); git_buf_free(&refnamebuf); return error; diff --git a/tests/refs/lookup.c b/tests/refs/lookup.c index d076e491f..456d0d2a8 100644 --- a/tests/refs/lookup.c +++ b/tests/refs/lookup.c @@ -58,3 +58,11 @@ void test_refs_lookup__namespace(void) error = git_reference_lookup(&ref, g_repo, "refs/heads/"); cl_assert_equal_i(error, GIT_EINVALIDSPEC); } + +void test_refs_lookup__dwim_notfound(void) +{ + git_reference *ref; + + cl_git_fail_with(GIT_ENOTFOUND, git_reference_dwim(&ref, g_repo, "idontexist")); + cl_assert_equal_s("no reference found for shorthand 'idontexist'", giterr_last()->message); +} From ba52879b46f3c615e488149b65d5e0eb18e6f7b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Mon, 11 Apr 2016 11:37:02 +0200 Subject: [PATCH 44/58] reset: use real ids for the tests This lets us run with strict object creation on. --- tests/reset/hard.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tests/reset/hard.c b/tests/reset/hard.c index 0c0af914c..e461f8093 100644 --- a/tests/reset/hard.c +++ b/tests/reset/hard.c @@ -122,9 +122,9 @@ static void unmerged_index_init(git_index *index, int entries) int write_theirs = 4; git_oid ancestor, ours, theirs; - git_oid_fromstr(&ancestor, "6bb0d9f700543ba3d318ba7075fc3bd696b4287b"); - git_oid_fromstr(&ours, "b19a1e93bec1317dc6097229e12afaffbfa74dc2"); - git_oid_fromstr(&theirs, "950b81b7eee953d050aa05a641f8e056c85dd1bd"); + git_oid_fromstr(&ancestor, "452e4244b5d083ddf0460acf1ecc74db9dcfa11a"); + git_oid_fromstr(&ours, "32504b727382542f9f089e24fddac5e78533e96c"); + git_oid_fromstr(&theirs, "061d42a44cacde5726057b67558821d95db96f19"); cl_git_rewritefile("status/conflicting_file", "conflicting file\n"); From b6130fe15e3a1a25378624c3523facb3fc3dc14f Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2016 11:50:11 -0400 Subject: [PATCH 45/58] refs::create: strict object creation on by default When we turned strict object creation validation on by default, we forgot to inform the refs::create tests of this. They, in fact, believed that strict object creation was off by default. As a result, their cleanup function went and turned strict object creation off for the remaining tests. --- tests/refs/create.c | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/tests/refs/create.c b/tests/refs/create.c index b96d0c90a..6d5a5f1f6 100644 --- a/tests/refs/create.c +++ b/tests/refs/create.c @@ -19,7 +19,7 @@ void test_refs_create__cleanup(void) { cl_git_sandbox_cleanup(); - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); } void test_refs_create__symbolic(void) @@ -122,7 +122,7 @@ void test_refs_create__oid(void) } /* Can by default create a reference that targets at an unknown id */ -void test_refs_create__oid_unknown_succeeds_by_default(void) +void test_refs_create__oid_unknown_succeeds_without_strict(void) { git_reference *new_reference, *looked_up_ref; git_oid id; @@ -131,6 +131,8 @@ void test_refs_create__oid_unknown_succeeds_by_default(void) git_oid_fromstr(&id, "deadbeef3f795b2b4353bcce3a527ad0a4f7f644"); + cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 0)); + /* Create and write the new object id reference */ cl_git_pass(git_reference_create(&new_reference, g_repo, new_head, &id, 0, NULL)); git_reference_free(new_reference); @@ -141,7 +143,7 @@ void test_refs_create__oid_unknown_succeeds_by_default(void) } /* Strict object enforcement enforces valid object id */ -void test_refs_create__oid_unknown_fails_strict_mode(void) +void test_refs_create__oid_unknown_fails_by_default(void) { git_reference *new_reference, *looked_up_ref; git_oid id; @@ -150,8 +152,6 @@ void test_refs_create__oid_unknown_fails_strict_mode(void) git_oid_fromstr(&id, "deadbeef3f795b2b4353bcce3a527ad0a4f7f644"); - cl_git_pass(git_libgit2_opts(GIT_OPT_ENABLE_STRICT_OBJECT_CREATION, 1)); - /* Create and write the new object id reference */ cl_git_fail(git_reference_create(&new_reference, g_repo, new_head, &id, 0, NULL)); From e8d5df9edc29196d7119189e5290ffd2df8e8b6c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 4 Mar 2016 14:51:16 +0100 Subject: [PATCH 46/58] config: show we write a spurious duplicated section header We should notice that we are in the correct section to add. This is a cosmetic bug, since replacing any of these settings does work. --- tests/config/write.c | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) diff --git a/tests/config/write.c b/tests/config/write.c index e634aa326..ac0272eac 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -695,3 +695,27 @@ void test_config_write__locking(void) git_config_free(cfg); } + +void test_config_write__repeated(void) +{ + const char *filename = "config-repeated"; + git_config *cfg; + git_buf result; + const char *expected = "[sample \"prefix\"]\n\ +\tsetting1 = someValue1\n\ +\tsetting2 = someValue2\n\ +\tsetting3 = someValue3\n\ +\tsetting4 = someValue4\n\ +"; + cl_git_pass(git_config_open_ondisk(&cfg, filename)); + cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting1", "someValue1")); + cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting2", "someValue2")); + cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting3", "someValue3")); + cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting4", "someValue4")); + + cl_git_pass(git_config_open_ondisk(&cfg, filename)); + + cl_git_pass(git_futils_readbuffer(&result, filename)); + cl_assert_equal_s(expected, result.ptr); + git_buf_free(&result); +} From a13c1ec2065bf2bd1afeb2f8fca0adf51ec48bfb Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 28 Mar 2016 11:13:51 -0400 Subject: [PATCH 47/58] config: don't write section header if we're in it If we hit the EOF while trying to write a new value, it may be that we're already in the section that we were looking for. If so, do not write a (duplicate) section header, just write the value. --- src/config_file.c | 13 +++++++++---- 1 file changed, 9 insertions(+), 4 deletions(-) diff --git a/src/config_file.c b/src/config_file.c index f49596cc0..ca4345cc7 100644 --- a/src/config_file.c +++ b/src/config_file.c @@ -1472,7 +1472,7 @@ static int config_parse( int (*on_section)(struct reader **reader, const char *current_section, const char *line, size_t line_len, void *data), int (*on_variable)(struct reader **reader, const char *current_section, char *var_name, char *var_value, const char *line, size_t line_len, void *data), int (*on_comment)(struct reader **reader, const char *line, size_t line_len, void *data), - int (*on_eof)(struct reader **reader, void *data), + int (*on_eof)(struct reader **reader, const char *current_section, void *data), void *data) { char *current_section = NULL, *var_name, *var_value, *line_start; @@ -1523,7 +1523,7 @@ static int config_parse( } if (on_eof) - result = on_eof(&reader, data); + result = on_eof(&reader, current_section, data); git__free(current_section); return result; @@ -1839,7 +1839,8 @@ static int write_on_comment(struct reader **reader, const char *line, size_t lin return write_line_to(&write_data->buffered_comment, line, line_len); } -static int write_on_eof(struct reader **reader, void *data) +static int write_on_eof( + struct reader **reader, const char *current_section, void *data) { struct write_data *write_data = (struct write_data *)data; int result = 0; @@ -1858,7 +1859,11 @@ static int write_on_eof(struct reader **reader, void *data) * value. */ if ((!write_data->preg || !write_data->preg_replaced) && write_data->value) { - if ((result = write_section(write_data->buf, write_data->section)) == 0) + /* write the section header unless we're already in it */ + if (!current_section || strcmp(current_section, write_data->section)) + result = write_section(write_data->buf, write_data->section); + + if (!result) result = write_value(write_data); } From 21d8832e74be0050d4b596b6a22a216ababdfaf7 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 28 Mar 2016 08:56:13 -0700 Subject: [PATCH 48/58] config::write::repeated: init our buffer --- tests/config/write.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/config/write.c b/tests/config/write.c index ac0272eac..e83cfb415 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -700,7 +700,7 @@ void test_config_write__repeated(void) { const char *filename = "config-repeated"; git_config *cfg; - git_buf result; + git_buf result = GIT_BUF_INIT; const char *expected = "[sample \"prefix\"]\n\ \tsetting1 = someValue1\n\ \tsetting2 = someValue2\n\ From 177890838a5d9165d621dfc32674ce5ecc41ee36 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Fri, 1 Apr 2016 14:33:42 +0200 Subject: [PATCH 49/58] ignore: don't use realpath to canonicalize path If we're looking for a symlink, realpath will give us the resolved path, which is not what we're after, but a canonicalized version of the path the user asked for. --- src/ignore.c | 14 +++++++++++--- tests/attr/ignore.c | 13 +++++++++++++ 2 files changed, 24 insertions(+), 3 deletions(-) diff --git a/src/ignore.c b/src/ignore.c index aedc1401e..ac2af4f58 100644 --- a/src/ignore.c +++ b/src/ignore.c @@ -263,10 +263,18 @@ int git_ignore__for_path( goto cleanup; /* given a unrooted path in a non-bare repo, resolve it */ - if (workdir && git_path_root(path) < 0) - error = git_path_find_dir(&ignores->dir, path, workdir); - else + if (workdir && git_path_root(path) < 0) { + git_buf local = GIT_BUF_INIT; + + if ((error = git_path_dirname_r(&local, path)) < 0 || + (error = git_path_resolve_relative(&local, 0)) < 0 || + (error = git_path_to_dir(&local)) < 0 || + (error = git_buf_joinpath(&ignores->dir, workdir, local.ptr)) < 0) + {;} /* Nothing, we just want to stop on the first error */ + git_buf_free(&local); + } else { error = git_buf_joinpath(&ignores->dir, path, ""); + } if (error < 0) goto cleanup; diff --git a/tests/attr/ignore.c b/tests/attr/ignore.c index 27fed2539..91bf984a1 100644 --- a/tests/attr/ignore.c +++ b/tests/attr/ignore.c @@ -252,3 +252,16 @@ void test_attr_ignore__dont_ignore_files_for_folder(void) if (cl_repo_get_bool(g_repo, "core.ignorecase")) assert_is_ignored(false, "dir/TeSt"); } + +void test_attr_ignore__symlink_to_outside(void) +{ +#ifdef GIT_WIN32 + cl_skip(); +#endif + + cl_git_rewritefile("attr/.gitignore", "symlink\n"); + cl_git_mkfile("target", "target"); + cl_git_pass(p_symlink("../target", "attr/symlink")); + assert_is_ignored(true, "symlink"); + assert_is_ignored(true, "lala/../symlink"); +} From 26f2cefb8164330714513e4a3502918fc854a3ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 20 Mar 2016 11:00:12 +0100 Subject: [PATCH 50/58] tree: re-use the id and filename in the odb object Instead of copying over the data into the individual entries, point to the originals, which are already in a format we can use. --- src/index.c | 2 +- src/iterator.c | 4 +- src/push.c | 12 ++--- src/submodule.c | 2 +- src/tree.c | 90 +++++++++++++++++++--------------- src/tree.h | 5 +- tests/diff/iterator.c | 2 +- tests/object/tree/attributes.c | 2 + 8 files changed, 67 insertions(+), 52 deletions(-) diff --git a/src/index.c b/src/index.c index 62aacf959..63e47965a 100644 --- a/src/index.c +++ b/src/index.c @@ -2836,7 +2836,7 @@ static int read_tree_cb( return -1; entry->mode = tentry->attr; - entry->id = tentry->oid; + git_oid_cpy(&entry->id, git_tree_entry_id(tentry)); /* look for corresponding old entry and copy data to new entry */ if (data->old_entries != NULL && diff --git a/src/iterator.c b/src/iterator.c index 024a97573..cb1ea6a87 100644 --- a/src/iterator.c +++ b/src/iterator.c @@ -458,7 +458,7 @@ static int tree_iterator__set_next(tree_iterator *ti, tree_iterator_frame *tf) /* try to load trees for items in [current,next) range */ if (!error && git_tree_entry__is_tree(te)) error = git_tree_lookup( - &tf->entries[tf->next]->tree, ti->base.repo, &te->oid); + &tf->entries[tf->next]->tree, ti->base.repo, te->oid); } if (tf->next > tf->current + 1) @@ -603,7 +603,7 @@ static int tree_iterator__update_entry(tree_iterator *ti) te = tf->entries[tf->current]->te; ti->entry.mode = te->attr; - git_oid_cpy(&ti->entry.id, &te->oid); + git_oid_cpy(&ti->entry.id, te->oid); ti->entry.path = tree_iterator__current_filename(ti, te); GITERR_CHECK_ALLOC(ti->entry.path); diff --git a/src/push.c b/src/push.c index 3c9fa2f1b..0747259c8 100644 --- a/src/push.c +++ b/src/push.c @@ -374,9 +374,9 @@ static int enqueue_object( case GIT_OBJ_COMMIT: return 0; case GIT_OBJ_TREE: - return git_packbuilder_insert_tree(pb, &entry->oid); + return git_packbuilder_insert_tree(pb, entry->oid); default: - return git_packbuilder_insert(pb, &entry->oid, entry->filename); + return git_packbuilder_insert(pb, entry->oid, entry->filename); } } @@ -396,7 +396,7 @@ static int queue_differences( const git_tree_entry *d_entry = git_tree_entry_byindex(delta, j); int cmp = 0; - if (!git_oid__cmp(&b_entry->oid, &d_entry->oid)) + if (!git_oid__cmp(b_entry->oid, d_entry->oid)) goto loop; cmp = strcmp(b_entry->filename, d_entry->filename); @@ -407,15 +407,15 @@ static int queue_differences( git_tree_entry__is_tree(b_entry) && git_tree_entry__is_tree(d_entry)) { /* Add the right-hand entry */ - if ((error = git_packbuilder_insert(pb, &d_entry->oid, + if ((error = git_packbuilder_insert(pb, d_entry->oid, d_entry->filename)) < 0) goto on_error; /* Acquire the subtrees and recurse */ if ((error = git_tree_lookup(&b_child, - git_tree_owner(base), &b_entry->oid)) < 0 || + git_tree_owner(base), b_entry->oid)) < 0 || (error = git_tree_lookup(&d_child, - git_tree_owner(delta), &d_entry->oid)) < 0 || + git_tree_owner(delta), d_entry->oid)) < 0 || (error = queue_differences(b_child, d_child, pb)) < 0) goto on_error; diff --git a/src/submodule.c b/src/submodule.c index 3f39b9ef0..c903cf939 100644 --- a/src/submodule.c +++ b/src/submodule.c @@ -1417,7 +1417,7 @@ static int submodule_update_head(git_submodule *submodule) git_tree_entry_bypath(&te, head, submodule->path) < 0) giterr_clear(); else - submodule_update_from_head_data(submodule, te->attr, &te->oid); + submodule_update_from_head_data(submodule, te->attr, git_tree_entry_id(te)); git_tree_entry_free(te); git_tree_free(head); diff --git a/src/tree.c b/src/tree.c index 48b9f121d..a6bd7d4fb 100644 --- a/src/tree.c +++ b/src/tree.c @@ -87,25 +87,40 @@ int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2) /** * Allocate either from the pool or from the system allocator */ -static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, size_t filename_len) +static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id) { git_tree_entry *entry = NULL; size_t tree_len; TREE_ENTRY_CHECK_NAMELEN(filename_len); + tree_len = sizeof(git_tree_entry); - if (GIT_ADD_SIZET_OVERFLOW(&tree_len, sizeof(git_tree_entry), filename_len) || - GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1)) + if (!pool && (GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, filename_len) || + GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) || + GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, GIT_OID_RAWSZ))) return NULL; - entry = pool ? git_pool_malloc(pool, tree_len) : - git__malloc(tree_len); + entry = pool ? git_pool_mallocz(pool, tree_len) : + git__calloc(1, tree_len); if (!entry) return NULL; - memset(entry, 0x0, sizeof(git_tree_entry)); - memcpy(entry->filename, filename, filename_len); - entry->filename[filename_len] = 0; + if (pool) { + entry->filename = filename; + entry->oid = (git_oid *) id; + } else { + char *filename_ptr; + void *id_ptr; + + filename_ptr = ((char *) entry) + sizeof(git_tree_entry); + memcpy(filename_ptr, filename, filename_len); + entry->filename = filename_ptr; + + id_ptr = filename_ptr + filename_len + 1; + git_oid_cpy(id_ptr, id); + entry->oid = id_ptr; + } + entry->filename_len = (uint16_t)filename_len; return entry; @@ -116,11 +131,11 @@ static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, si * it. This is useful when reading trees, so we don't allocate a ton * of small strings but can use the pool. */ -static git_tree_entry *alloc_entry_pooled(git_pool *pool, const char *filename, size_t filename_len) +static git_tree_entry *alloc_entry_pooled(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id) { git_tree_entry *entry = NULL; - if (!(entry = alloc_entry_base(pool, filename, filename_len))) + if (!(entry = alloc_entry_base(pool, filename, filename_len, id))) return NULL; entry->pooled = true; @@ -130,7 +145,8 @@ static git_tree_entry *alloc_entry_pooled(git_pool *pool, const char *filename, static git_tree_entry *alloc_entry(const char *filename) { - return alloc_entry_base(NULL, filename, strlen(filename)); + git_oid dummy_id = { 0 }; + return alloc_entry_base(NULL, filename, strlen(filename), &dummy_id); } struct tree_key_search { @@ -242,22 +258,12 @@ void git_tree_entry_free(git_tree_entry *entry) int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source) { - size_t total_size; - git_tree_entry *copy; - assert(source); - GITERR_CHECK_ALLOC_ADD(&total_size, sizeof(git_tree_entry), source->filename_len); - GITERR_CHECK_ALLOC_ADD(&total_size, total_size, 1); + *dest = alloc_entry_base(NULL, source->filename, source->filename_len, source->oid); + if (*dest == NULL) + return -1; - copy = git__malloc(total_size); - GITERR_CHECK_ALLOC(copy); - - memcpy(copy, source, total_size); - - copy->pooled = 0; - - *dest = copy; return 0; } @@ -270,6 +276,7 @@ void git_tree__free(void *_tree) git_vector_foreach(&tree->entries, i, e) git_tree_entry_free(e); + git_odb_object_free(tree->odb_obj); git_vector_free(&tree->entries); git_pool_clear(&tree->pool); git__free(tree); @@ -294,7 +301,7 @@ const char *git_tree_entry_name(const git_tree_entry *entry) const git_oid *git_tree_entry_id(const git_tree_entry *entry) { assert(entry); - return &entry->oid; + return entry->oid; } git_otype git_tree_entry_type(const git_tree_entry *entry) @@ -315,7 +322,7 @@ int git_tree_entry_to_object( const git_tree_entry *entry) { assert(entry && object_out); - return git_object_lookup(object_out, repo, &entry->oid, GIT_OBJ_ANY); + return git_object_lookup(object_out, repo, entry->oid, GIT_OBJ_ANY); } static const git_tree_entry *entry_fromname( @@ -356,7 +363,7 @@ const git_tree_entry *git_tree_entry_byid( assert(tree); git_vector_foreach(&tree->entries, i, e) { - if (memcmp(&e->oid.id, &id->id, sizeof(id->id)) == 0) + if (memcmp(&e->oid->id, &id->id, sizeof(id->id)) == 0) return e; } @@ -444,8 +451,14 @@ static int parse_mode(unsigned int *modep, const char *buffer, const char **buff int git_tree__parse(void *_tree, git_odb_object *odb_obj) { git_tree *tree = _tree; - const char *buffer = git_odb_object_data(odb_obj); - const char *buffer_end = buffer + git_odb_object_size(odb_obj); + const char *buffer; + const char *buffer_end; + + if (git_odb_object_dup(&tree->odb_obj, odb_obj) < 0) + return -1; + + buffer = git_odb_object_data(tree->odb_obj); + buffer_end = buffer + git_odb_object_size(tree->odb_obj); git_pool_init(&tree->pool, 1); if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) @@ -466,7 +479,9 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) filename_len = nul - buffer; /** Allocate the entry and store it in the entries vector */ { - entry = alloc_entry_pooled(&tree->pool, buffer, filename_len); + /* Jump to the ID just after the path */ + const void *oid_ptr = buffer + filename_len + 1; + entry = alloc_entry_pooled(&tree->pool, buffer, filename_len, oid_ptr); GITERR_CHECK_ALLOC(entry); if (git_vector_insert(&tree->entries, entry) < 0) @@ -475,10 +490,7 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) entry->attr = attr; } - /* Advance to the ID just after the path */ buffer += filename_len + 1; - - git_oid_fromraw(&entry->oid, (const unsigned char *)buffer); buffer += GIT_OID_RAWSZ; } @@ -520,7 +532,7 @@ static int append_entry( entry = alloc_entry(filename); GITERR_CHECK_ALLOC(entry); - git_oid_cpy(&entry->oid, id); + git_oid_cpy(entry->oid, id); entry->attr = (uint16_t)filemode; git_strmap_insert(bld->map, entry->filename, entry, error); @@ -712,7 +724,7 @@ int git_treebuilder_new( git_vector_foreach(&source->entries, i, entry_src) { if (append_entry( bld, entry_src->filename, - &entry_src->oid, + entry_src->oid, entry_src->attr) < 0) goto on_error; } @@ -777,7 +789,7 @@ int git_treebuilder_insert( } } - git_oid_cpy(&entry->oid, id); + git_oid_cpy(entry->oid, id); entry->attr = filemode; if (entry_out) @@ -848,7 +860,7 @@ int git_treebuilder_write(git_oid *oid, git_treebuilder *bld) git_buf_printf(&tree, "%o ", entry->attr); git_buf_put(&tree, entry->filename, entry->filename_len + 1); - git_buf_put(&tree, (char *)entry->oid.id, GIT_OID_RAWSZ); + git_buf_put(&tree, (char *)entry->oid->id, GIT_OID_RAWSZ); if (git_buf_oom(&tree)) error = -1; @@ -960,7 +972,7 @@ int git_tree_entry_bypath( return git_tree_entry_dup(entry_out, entry); } - if (git_tree_lookup(&subtree, root->object.repo, &entry->oid) < 0) + if (git_tree_lookup(&subtree, root->object.repo, entry->oid) < 0) return -1; error = git_tree_entry_bypath( @@ -1001,7 +1013,7 @@ static int tree_walk( git_tree *subtree; size_t path_len = git_buf_len(path); - error = git_tree_lookup(&subtree, tree->object.repo, &entry->oid); + error = git_tree_lookup(&subtree, tree->object.repo, entry->oid); if (error < 0) break; diff --git a/src/tree.h b/src/tree.h index 914d788c8..00b27ae41 100644 --- a/src/tree.h +++ b/src/tree.h @@ -17,13 +17,14 @@ struct git_tree_entry { uint16_t attr; uint16_t filename_len; - git_oid oid; + git_oid *oid; bool pooled; - char filename[GIT_FLEX_ARRAY]; + const char *filename; }; struct git_tree { git_object object; + git_odb_object *odb_obj; git_vector entries; git_pool pool; }; diff --git a/tests/diff/iterator.c b/tests/diff/iterator.c index 8417e8ed4..25a23eda7 100644 --- a/tests/diff/iterator.c +++ b/tests/diff/iterator.c @@ -268,7 +268,7 @@ static void check_tree_entry( cl_git_pass(git_iterator_current_tree_entry(&te, i)); cl_assert(te); - cl_assert(git_oid_streq(&te->oid, oid) == 0); + cl_assert(git_oid_streq(te->oid, oid) == 0); cl_git_pass(git_iterator_current(&ie, i)); cl_git_pass(git_buf_sets(&path, ie->path)); diff --git a/tests/object/tree/attributes.c b/tests/object/tree/attributes.c index 413514c48..8654dfa31 100644 --- a/tests/object/tree/attributes.c +++ b/tests/object/tree/attributes.c @@ -82,6 +82,7 @@ void test_object_tree_attributes__normalize_attributes_when_creating_a_tree_from cl_git_pass(git_treebuilder_new(&builder, repo, tree)); entry = git_treebuilder_get(builder, "old_mode.txt"); + cl_assert(entry != NULL); cl_assert_equal_i( GIT_FILEMODE_BLOB, git_tree_entry_filemode(entry)); @@ -92,6 +93,7 @@ void test_object_tree_attributes__normalize_attributes_when_creating_a_tree_from cl_git_pass(git_tree_lookup(&tree, repo, &tid2)); entry = git_tree_entry_byname(tree, "old_mode.txt"); + cl_assert(entry != NULL); cl_assert_equal_i( GIT_FILEMODE_BLOB, git_tree_entry_filemode(entry)); From 13ebf7bdbc70a95c1405a03729176acae7623860 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Sun, 20 Mar 2016 12:01:45 +0100 Subject: [PATCH 51/58] tree: store the entries in a growable array Take advantage of the constant size of tree-owned arrays and store them in an array instead of a pool. This still lets us free them all at once but lets the system allocator do the work of fitting them in. --- src/tree.c | 93 +++++++++++++++++++++--------------------------------- src/tree.h | 5 ++- 2 files changed, 38 insertions(+), 60 deletions(-) diff --git a/src/tree.c b/src/tree.c index a6bd7d4fb..1ba484918 100644 --- a/src/tree.c +++ b/src/tree.c @@ -85,30 +85,26 @@ int git_tree_entry_icmp(const git_tree_entry *e1, const git_tree_entry *e2) } /** - * Allocate either from the pool or from the system allocator + * Allocate a new self-contained entry, with enough space after it to + * store the filename and the id. */ -static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id) +static git_tree_entry *alloc_entry(const char *filename, size_t filename_len, const git_oid *id) { git_tree_entry *entry = NULL; size_t tree_len; TREE_ENTRY_CHECK_NAMELEN(filename_len); - tree_len = sizeof(git_tree_entry); - if (!pool && (GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, filename_len) || - GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) || - GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, GIT_OID_RAWSZ))) + if (GIT_ADD_SIZET_OVERFLOW(&tree_len, sizeof(git_tree_entry), filename_len) || + GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, 1) || + GIT_ADD_SIZET_OVERFLOW(&tree_len, tree_len, GIT_OID_RAWSZ)) return NULL; - entry = pool ? git_pool_mallocz(pool, tree_len) : - git__calloc(1, tree_len); + entry = git__calloc(1, tree_len); if (!entry) return NULL; - if (pool) { - entry->filename = filename; - entry->oid = (git_oid *) id; - } else { + { char *filename_ptr; void *id_ptr; @@ -126,29 +122,6 @@ static git_tree_entry *alloc_entry_base(git_pool *pool, const char *filename, si return entry; } -/** - * Allocate a tree entry, using the poolin the tree which owns - * it. This is useful when reading trees, so we don't allocate a ton - * of small strings but can use the pool. - */ -static git_tree_entry *alloc_entry_pooled(git_pool *pool, const char *filename, size_t filename_len, const git_oid *id) -{ - git_tree_entry *entry = NULL; - - if (!(entry = alloc_entry_base(pool, filename, filename_len, id))) - return NULL; - - entry->pooled = true; - - return entry; -} - -static git_tree_entry *alloc_entry(const char *filename) -{ - git_oid dummy_id = { 0 }; - return alloc_entry_base(NULL, filename, strlen(filename), &dummy_id); -} - struct tree_key_search { const char *filename; uint16_t filename_len; @@ -250,7 +223,7 @@ static int tree_key_search( void git_tree_entry_free(git_tree_entry *entry) { - if (entry == NULL || entry->pooled) + if (entry == NULL) return; git__free(entry); @@ -258,27 +231,27 @@ void git_tree_entry_free(git_tree_entry *entry) int git_tree_entry_dup(git_tree_entry **dest, const git_tree_entry *source) { + git_tree_entry *cpy; + assert(source); - *dest = alloc_entry_base(NULL, source->filename, source->filename_len, source->oid); - if (*dest == NULL) + cpy = alloc_entry(source->filename, source->filename_len, source->oid); + if (cpy == NULL) return -1; + cpy->attr = source->attr; + + *dest = cpy; return 0; } void git_tree__free(void *_tree) { git_tree *tree = _tree; - size_t i; - git_tree_entry *e; - - git_vector_foreach(&tree->entries, i, e) - git_tree_entry_free(e); git_odb_object_free(tree->odb_obj); git_vector_free(&tree->entries); - git_pool_clear(&tree->pool); + git_array_clear(tree->entries_arr); git__free(tree); } @@ -450,6 +423,7 @@ static int parse_mode(unsigned int *modep, const char *buffer, const char **buff int git_tree__parse(void *_tree, git_odb_object *odb_obj) { + size_t i; git_tree *tree = _tree; const char *buffer; const char *buffer_end; @@ -460,9 +434,8 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) buffer = git_odb_object_data(tree->odb_obj); buffer_end = buffer + git_odb_object_size(tree->odb_obj); - git_pool_init(&tree->pool, 1); - if (git_vector_init(&tree->entries, DEFAULT_TREE_SIZE, entry_sort_cmp) < 0) - return -1; + git_array_init_to_size(tree->entries_arr, DEFAULT_TREE_SIZE); + GITERR_CHECK_ARRAY(tree->entries_arr); while (buffer < buffer_end) { git_tree_entry *entry; @@ -479,21 +452,28 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) filename_len = nul - buffer; /** Allocate the entry and store it in the entries vector */ { - /* Jump to the ID just after the path */ - const void *oid_ptr = buffer + filename_len + 1; - entry = alloc_entry_pooled(&tree->pool, buffer, filename_len, oid_ptr); + entry = git_array_alloc(tree->entries_arr); GITERR_CHECK_ALLOC(entry); - if (git_vector_insert(&tree->entries, entry) < 0) - return -1; - entry->attr = attr; + entry->filename_len = filename_len; + entry->filename = buffer; + entry->oid = (git_oid *) ((char *) buffer + filename_len + 1); } buffer += filename_len + 1; buffer += GIT_OID_RAWSZ; } + /* Add the entries to the vector here, as we may reallocate during the loop */ + if (git_vector_init(&tree->entries, tree->entries_arr.size, entry_sort_cmp) < 0) + return -1; + + for (i = 0; i < tree->entries_arr.size; i++) { + if (git_vector_insert(&tree->entries, git_array_get(tree->entries_arr, i)) < 0) + return -1; + } + /* The tree is sorted by definition. Bad inputs give bad outputs */ tree->entries.flags |= GIT_VECTOR_SORTED; @@ -529,10 +509,9 @@ static int append_entry( if (!valid_entry_name(bld->repo, filename)) return tree_error("Failed to insert entry. Invalid name for a tree entry", filename); - entry = alloc_entry(filename); + entry = alloc_entry(filename, strlen(filename), id); GITERR_CHECK_ALLOC(entry); - git_oid_cpy(entry->oid, id); entry->attr = (uint16_t)filemode; git_strmap_insert(bld->map, entry->filename, entry, error); @@ -776,8 +755,9 @@ int git_treebuilder_insert( pos = git_strmap_lookup_index(bld->map, filename); if (git_strmap_valid_index(bld->map, pos)) { entry = git_strmap_value_at(bld->map, pos); + git_oid_cpy((git_oid *) entry->oid, id); } else { - entry = alloc_entry(filename); + entry = alloc_entry(filename, strlen(filename), id); GITERR_CHECK_ALLOC(entry); git_strmap_insert(bld->map, entry->filename, entry, error); @@ -789,7 +769,6 @@ int git_treebuilder_insert( } } - git_oid_cpy(entry->oid, id); entry->attr = filemode; if (entry_out) diff --git a/src/tree.h b/src/tree.h index 00b27ae41..a108d964c 100644 --- a/src/tree.h +++ b/src/tree.h @@ -17,16 +17,15 @@ struct git_tree_entry { uint16_t attr; uint16_t filename_len; - git_oid *oid; - bool pooled; + const git_oid *oid; const char *filename; }; struct git_tree { git_object object; git_odb_object *odb_obj; + git_array_t(git_tree_entry) entries_arr; git_vector entries; - git_pool pool; }; struct git_treebuilder { From af753aba14db97dbead950263615069701aa73a9 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Tue, 22 Mar 2016 00:18:44 -0400 Subject: [PATCH 52/58] tree: drop the now-unnecessary entries vector Remove the now-unnecessary entries vector. Add `git_array_search` to binary search through an array to accomplish this. --- src/array.h | 40 +++++++++++++++++++++++++ src/tree.c | 73 ++++++++++++++++++---------------------------- src/tree.h | 3 +- tests/core/array.c | 55 ++++++++++++++++++++++++++++++++++ 4 files changed, 125 insertions(+), 46 deletions(-) create mode 100644 tests/core/array.c diff --git a/src/array.h b/src/array.h index 7cd9b7153..490e6be20 100644 --- a/src/array.h +++ b/src/array.h @@ -82,4 +82,44 @@ on_oom: #define git_array_valid_index(a, i) ((i) < (a).size) +#define git_array_foreach(a, i, element) \ + for ((i) = 0; (i) < (a).size && ((element) = &(a).ptr[(i)]); (i)++) + + +GIT_INLINE(int) git_array__search( + size_t *out, + void *array_ptr, + size_t item_size, + size_t array_len, + int (*compare)(const void *, const void *), + const void *key) +{ + size_t lim; + unsigned char *part, *array = array_ptr, *base = array_ptr; + int cmp; + + for (lim = array_len; lim != 0; lim >>= 1) { + part = base + (lim >> 1) * item_size; + cmp = (*compare)(key, part); + + if (cmp == 0) { + base = part; + break; + } + if (cmp > 0) { /* key > p; take right partition */ + base = part + 1 * item_size; + lim--; + } /* else take left partition */ + } + + if (out) + *out = (base - array) / item_size; + + return (cmp == 0) ? 0 : GIT_ENOTFOUND; +} + +#define git_array_search(out, a, cmp, key) \ + git_array__search(out, (a).ptr, sizeof(*(a).ptr), (a).size, \ + (cmp), (key)) + #endif diff --git a/src/tree.c b/src/tree.c index 1ba484918..c1bd4597f 100644 --- a/src/tree.c +++ b/src/tree.c @@ -163,7 +163,10 @@ static int homing_search_cmp(const void *key, const void *array_member) * around the area for our target file. */ static int tree_key_search( - size_t *at_pos, git_vector *entries, const char *filename, size_t filename_len) + size_t *at_pos, + const git_tree *tree, + const char *filename, + size_t filename_len) { struct tree_key_search ksearch; const git_tree_entry *entry; @@ -176,13 +179,15 @@ static int tree_key_search( /* Initial homing search; find an entry on the tree with * the same prefix as the filename we're looking for */ - if (git_vector_bsearch2(&homing, entries, &homing_search_cmp, &ksearch) < 0) + + if (git_array_search(&homing, + tree->entries, &homing_search_cmp, &ksearch) < 0) return GIT_ENOTFOUND; /* just a signal error; not passed back to user */ /* We found a common prefix. Look forward as long as * there are entries that share the common prefix */ - for (i = homing; i < entries->length; ++i) { - entry = entries->contents[i]; + for (i = homing; i < tree->entries.size; ++i) { + entry = git_array_get(tree->entries, i); if (homing_search_cmp(&ksearch, entry) < 0) break; @@ -202,7 +207,7 @@ static int tree_key_search( i = homing - 1; do { - entry = entries->contents[i]; + entry = git_array_get(tree->entries, i); if (homing_search_cmp(&ksearch, entry) > 0) break; @@ -250,8 +255,7 @@ void git_tree__free(void *_tree) git_tree *tree = _tree; git_odb_object_free(tree->odb_obj); - git_vector_free(&tree->entries); - git_array_clear(tree->entries_arr); + git_array_clear(tree->entries); git__free(tree); } @@ -303,13 +307,10 @@ static const git_tree_entry *entry_fromname( { size_t idx; - /* be safe when we cast away constness - i.e. don't trigger a sort */ - assert(git_vector_is_sorted(&tree->entries)); - - if (tree_key_search(&idx, (git_vector *)&tree->entries, name, name_len) < 0) + if (tree_key_search(&idx, tree, name, name_len) < 0) return NULL; - return git_vector_get(&tree->entries, idx); + return git_array_get(tree->entries, idx); } const git_tree_entry *git_tree_entry_byname( @@ -324,7 +325,7 @@ const git_tree_entry *git_tree_entry_byindex( const git_tree *tree, size_t idx) { assert(tree); - return git_vector_get(&tree->entries, idx); + return git_array_get(tree->entries, idx); } const git_tree_entry *git_tree_entry_byid( @@ -335,7 +336,7 @@ const git_tree_entry *git_tree_entry_byid( assert(tree); - git_vector_foreach(&tree->entries, i, e) { + git_array_foreach(tree->entries, i, e) { if (memcmp(&e->oid->id, &id->id, sizeof(id->id)) == 0) return e; } @@ -345,7 +346,6 @@ const git_tree_entry *git_tree_entry_byid( int git_tree__prefix_position(const git_tree *tree, const char *path) { - const git_vector *entries = &tree->entries; struct tree_key_search ksearch; size_t at_pos, path_len; @@ -358,21 +358,20 @@ int git_tree__prefix_position(const git_tree *tree, const char *path) ksearch.filename = path; ksearch.filename_len = (uint16_t)path_len; - /* be safe when we cast away constness - i.e. don't trigger a sort */ - assert(git_vector_is_sorted(&tree->entries)); - /* Find tree entry with appropriate prefix */ - git_vector_bsearch2( - &at_pos, (git_vector *)entries, &homing_search_cmp, &ksearch); + git_array_search( + &at_pos, tree->entries, &homing_search_cmp, &ksearch); - for (; at_pos < entries->length; ++at_pos) { - const git_tree_entry *entry = entries->contents[at_pos]; + for (; at_pos < tree->entries.size; ++at_pos) { + const git_tree_entry *entry = git_array_get(tree->entries, at_pos); if (homing_search_cmp(&ksearch, entry) < 0) break; } for (; at_pos > 0; --at_pos) { - const git_tree_entry *entry = entries->contents[at_pos - 1]; + const git_tree_entry *entry = + git_array_get(tree->entries, at_pos - 1); + if (homing_search_cmp(&ksearch, entry) > 0) break; } @@ -383,7 +382,7 @@ int git_tree__prefix_position(const git_tree *tree, const char *path) size_t git_tree_entrycount(const git_tree *tree) { assert(tree); - return tree->entries.length; + return tree->entries.size; } unsigned int git_treebuilder_entrycount(git_treebuilder *bld) @@ -423,7 +422,6 @@ static int parse_mode(unsigned int *modep, const char *buffer, const char **buff int git_tree__parse(void *_tree, git_odb_object *odb_obj) { - size_t i; git_tree *tree = _tree; const char *buffer; const char *buffer_end; @@ -434,8 +432,8 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) buffer = git_odb_object_data(tree->odb_obj); buffer_end = buffer + git_odb_object_size(tree->odb_obj); - git_array_init_to_size(tree->entries_arr, DEFAULT_TREE_SIZE); - GITERR_CHECK_ARRAY(tree->entries_arr); + git_array_init_to_size(tree->entries, DEFAULT_TREE_SIZE); + GITERR_CHECK_ARRAY(tree->entries); while (buffer < buffer_end) { git_tree_entry *entry; @@ -450,9 +448,9 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) return tree_error("Failed to parse tree. Object is corrupted", NULL); filename_len = nul - buffer; - /** Allocate the entry and store it in the entries vector */ + /* Allocate the entry */ { - entry = git_array_alloc(tree->entries_arr); + entry = git_array_alloc(tree->entries); GITERR_CHECK_ALLOC(entry); entry->attr = attr; @@ -465,18 +463,6 @@ int git_tree__parse(void *_tree, git_odb_object *odb_obj) buffer += GIT_OID_RAWSZ; } - /* Add the entries to the vector here, as we may reallocate during the loop */ - if (git_vector_init(&tree->entries, tree->entries_arr.size, entry_sort_cmp) < 0) - return -1; - - for (i = 0; i < tree->entries_arr.size; i++) { - if (git_vector_insert(&tree->entries, git_array_get(tree->entries_arr, i)) < 0) - return -1; - } - - /* The tree is sorted by definition. Bad inputs give bad outputs */ - tree->entries.flags |= GIT_VECTOR_SORTED; - return 0; } @@ -700,7 +686,7 @@ int git_treebuilder_new( if (source != NULL) { git_tree_entry *entry_src; - git_vector_foreach(&source->entries, i, entry_src) { + git_array_foreach(source->entries, i, entry_src) { if (append_entry( bld, entry_src->filename, entry_src->oid, @@ -845,7 +831,6 @@ int git_treebuilder_write(git_oid *oid, git_treebuilder *bld) error = -1; } - git_vector_free(&entries); if (!error && !(error = git_repository_odb__weakptr(&odb, bld->repo))) @@ -975,7 +960,7 @@ static int tree_walk( size_t i; const git_tree_entry *entry; - git_vector_foreach(&tree->entries, i, entry) { + git_array_foreach(tree->entries, i, entry) { if (preorder) { error = callback(path->ptr, entry, payload); if (error < 0) { /* negative value stops iteration */ diff --git a/src/tree.h b/src/tree.h index a108d964c..5e7a66e04 100644 --- a/src/tree.h +++ b/src/tree.h @@ -24,8 +24,7 @@ struct git_tree_entry { struct git_tree { git_object object; git_odb_object *odb_obj; - git_array_t(git_tree_entry) entries_arr; - git_vector entries; + git_array_t(git_tree_entry) entries; }; struct git_treebuilder { diff --git a/tests/core/array.c b/tests/core/array.c new file mode 100644 index 000000000..375cc8df3 --- /dev/null +++ b/tests/core/array.c @@ -0,0 +1,55 @@ +#include "clar_libgit2.h" +#include "array.h" + +static int int_lookup(const void *k, const void *a) +{ + const int *one = (const int *)k; + int *two = (int *)a; + + return *one - *two; +} + +#define expect_pos(k, n, ret) \ + key = (k); \ + cl_assert_equal_i((ret), \ + git_array_search(&p, integers, int_lookup, &key)); \ + cl_assert_equal_i((n), p); + +void test_core_array__bsearch2(void) +{ + git_array_t(int) integers = GIT_ARRAY_INIT; + int *i, key; + size_t p; + + i = git_array_alloc(integers); *i = 2; + i = git_array_alloc(integers); *i = 3; + i = git_array_alloc(integers); *i = 5; + i = git_array_alloc(integers); *i = 7; + i = git_array_alloc(integers); *i = 7; + i = git_array_alloc(integers); *i = 8; + i = git_array_alloc(integers); *i = 13; + i = git_array_alloc(integers); *i = 21; + i = git_array_alloc(integers); *i = 25; + i = git_array_alloc(integers); *i = 42; + i = git_array_alloc(integers); *i = 69; + i = git_array_alloc(integers); *i = 121; + i = git_array_alloc(integers); *i = 256; + i = git_array_alloc(integers); *i = 512; + i = git_array_alloc(integers); *i = 513; + i = git_array_alloc(integers); *i = 514; + i = git_array_alloc(integers); *i = 516; + i = git_array_alloc(integers); *i = 516; + i = git_array_alloc(integers); *i = 517; + + /* value to search for, expected position, return code */ + expect_pos(3, 1, GIT_OK); + expect_pos(2, 0, GIT_OK); + expect_pos(1, 0, GIT_ENOTFOUND); + expect_pos(25, 8, GIT_OK); + expect_pos(26, 9, GIT_ENOTFOUND); + expect_pos(42, 9, GIT_OK); + expect_pos(50, 10, GIT_ENOTFOUND); + expect_pos(68, 10, GIT_ENOTFOUND); + expect_pos(256, 12, GIT_OK); +} + From 1d59c85aade2260882cc136c3720cc74c3df4382 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 17 Mar 2016 00:47:50 -0400 Subject: [PATCH 53/58] status: update test to include valid OID --- tests/status/worktree.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/status/worktree.c b/tests/status/worktree.c index fc4afc6be..6b823785c 100644 --- a/tests/status/worktree.c +++ b/tests/status/worktree.c @@ -657,7 +657,7 @@ void test_status_worktree__conflict_has_no_oid(void) entry.mode = 0100644; entry.path = "modified_file"; - git_oid_fromstr(&entry.id, "deadbeefdeadbeefdeadbeefdeadbeefdeadbeef"); + git_oid_fromstr(&entry.id, "452e4244b5d083ddf0460acf1ecc74db9dcfa11a"); cl_git_pass(git_repository_index(&index, repo)); cl_git_pass(git_index_conflict_add(index, &entry, &entry, &entry)); From 5cc7a5c7b389066ab7398006819ab31b67b3a187 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 11 Apr 2016 13:39:31 -0400 Subject: [PATCH 54/58] tests: skip the unreadable file tests as root When running as root, skip the unreadable file tests, because, well, they're probably _not_ unreadable to root unless you've got some crazy NSA clearance-level honoring operating system shit going on. --- tests/repo/iterator.c | 5 +++++ tests/status/worktree.c | 3 +++ 2 files changed, 8 insertions(+) diff --git a/tests/repo/iterator.c b/tests/repo/iterator.c index c18e24a4f..6b5795b9c 100644 --- a/tests/repo/iterator.c +++ b/tests/repo/iterator.c @@ -1020,6 +1020,11 @@ void test_repo_iterator__unreadable_dir(void) if (!cl_is_chmod_supported()) return; +#ifndef GIT_WIN32 + if (geteuid() == 0) + cl_skip(); +#endif + g_repo = cl_git_sandbox_init("empty_standard_repo"); cl_must_pass(p_mkdir("empty_standard_repo/r", 0777)); diff --git a/tests/status/worktree.c b/tests/status/worktree.c index 6b823785c..5d3b4d55e 100644 --- a/tests/status/worktree.c +++ b/tests/status/worktree.c @@ -1006,6 +1006,9 @@ void test_status_worktree__unreadable(void) git_status_options opts = GIT_STATUS_OPTIONS_INIT; status_entry_counts counts = {0}; + if (geteuid() == 0) + cl_skip(); + /* Create directory with no read permission */ cl_git_pass(git_futils_mkdir_r("empty_standard_repo/no_permission", 0777)); cl_git_mkfile("empty_standard_repo/no_permission/foo", "dummy"); From 2c1bc36d56e3d07feb098bd805beb7078acdd5ae Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Carlos=20Mart=C3=ADn=20Nieto?= Date: Tue, 29 Mar 2016 14:47:31 +0200 Subject: [PATCH 55/58] Plug a few leaks --- src/tree.c | 2 ++ tests/core/array.c | 2 ++ 2 files changed, 4 insertions(+) diff --git a/src/tree.c b/src/tree.c index c1bd4597f..6ce460c6d 100644 --- a/src/tree.c +++ b/src/tree.c @@ -837,6 +837,8 @@ int git_treebuilder_write(git_oid *oid, git_treebuilder *bld) error = git_odb_write(oid, odb, tree.ptr, tree.size, GIT_OBJ_TREE); git_buf_free(&tree); + git_vector_free(&entries); + return error; } diff --git a/tests/core/array.c b/tests/core/array.c index 375cc8df3..8e626a506 100644 --- a/tests/core/array.c +++ b/tests/core/array.c @@ -51,5 +51,7 @@ void test_core_array__bsearch2(void) expect_pos(50, 10, GIT_ENOTFOUND); expect_pos(68, 10, GIT_ENOTFOUND); expect_pos(256, 12, GIT_OK); + + git_array_clear(integers); } From 6a35e74cb3a2f6cb1640c2eec1c99665b40d4df7 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Wed, 30 Mar 2016 17:47:05 -0400 Subject: [PATCH 56/58] leaks: fix some leaks in the tests --- tests/config/write.c | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests/config/write.c b/tests/config/write.c index e83cfb415..56ef2e9fb 100644 --- a/tests/config/write.c +++ b/tests/config/write.c @@ -712,10 +712,13 @@ void test_config_write__repeated(void) cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting2", "someValue2")); cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting3", "someValue3")); cl_git_pass(git_config_set_string(cfg, "sample.prefix.setting4", "someValue4")); + git_config_free(cfg); cl_git_pass(git_config_open_ondisk(&cfg, filename)); cl_git_pass(git_futils_readbuffer(&result, filename)); cl_assert_equal_s(expected, result.ptr); git_buf_free(&result); + + git_config_free(cfg); } From ccfacb8bb18e114716ca2034deb6d6b2873313c4 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Thu, 31 Mar 2016 10:43:57 -0400 Subject: [PATCH 57/58] leaks: call `xdl_free_classifier` --- src/xdiff/xprepare.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/xdiff/xprepare.c b/src/xdiff/xprepare.c index 3183d50eb..13b55aba7 100644 --- a/src/xdiff/xprepare.c +++ b/src/xdiff/xprepare.c @@ -305,7 +305,7 @@ int xdl_prepare_env(mmfile_t *mf1, mmfile_t *mf2, xpparam_t const *xpp, return -1; } - if (XDF_DIFF_ALG((xpp->flags) & XDF_HISTOGRAM_DIFF)) + if (XDF_DIFF_ALG(xpp->flags) != XDF_HISTOGRAM_DIFF) xdl_free_classifier(&cf); return 0; From 8edadbf97889fbd80289a72209fdd485ddea3521 Mon Sep 17 00:00:00 2001 From: Edward Thomson Date: Mon, 7 Mar 2016 17:37:17 -0500 Subject: [PATCH 58/58] index::racy: force racy entry Instead of hoping that we can get a racy entry by going real fast and praying real hard, just create a racy entry. --- tests/index/racy.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/tests/index/racy.c b/tests/index/racy.c index 68aa46007..1768f5efd 100644 --- a/tests/index/racy.c +++ b/tests/index/racy.c @@ -105,8 +105,8 @@ static void setup_race(void) { git_buf path = GIT_BUF_INIT; git_index *index; - const git_index_entry *entry; - int i, found_race = 0; + git_index_entry *entry; + struct stat st; /* Make sure we do have a timestamp */ cl_git_pass(git_repository_index__weakptr(&index, g_repo)); @@ -114,27 +114,20 @@ static void setup_race(void) cl_git_pass(git_buf_joinpath(&path, git_repository_workdir(g_repo), "A")); - /* Make sure writing the file, adding and rewriting happen in the same second */ - for (i = 0; i < 10; i++) { - struct stat st; - cl_git_mkfile(path.ptr, "A"); + cl_git_mkfile(path.ptr, "A"); + cl_git_pass(git_index_add_bypath(index, "A")); - cl_git_pass(git_index_add_bypath(index, "A")); - cl_git_mkfile(path.ptr, "B"); - cl_git_pass(git_index_write(index)); + cl_git_mkfile(path.ptr, "B"); + cl_git_pass(git_index_write(index)); - cl_git_mkfile(path.ptr, ""); + cl_git_mkfile(path.ptr, ""); - cl_git_pass(p_stat(path.ptr, &st)); - cl_assert(entry = git_index_get_bypath(index, "A", 0)); - if (entry->mtime.seconds == (int32_t) st.st_mtime) { - found_race = 1; - break; - } - } + cl_git_pass(p_stat(path.ptr, &st)); + cl_assert(entry = (git_index_entry *)git_index_get_bypath(index, "A", 0)); - if (!found_race) - cl_fail("failed to find race after 10 attempts"); + /* force a race */ + entry->mtime.seconds = st.st_mtime; + entry->mtime.nanoseconds = st.st_mtime_nsec; git_buf_free(&path); }