diff options
author | Edward Thomson <ethomson@github.com> | 2016-02-25 12:09:49 -0500 |
---|---|---|
committer | Edward Thomson <ethomson@github.com> | 2016-02-25 12:09:49 -0500 |
commit | 0d9a7498c5f8852a60ffaf4d1c6b53c919193694 (patch) | |
tree | 5f96713af0b14f4265dffd783755169e9c932851 | |
parent | fd129f28f1c4662b9315a264f495c3ed7a9cfb50 (diff) | |
parent | 32f0798413f83cbd1c22e11d81eeb9f664181ec9 (diff) | |
download | libgit2-0d9a7498c5f8852a60ffaf4d1c6b53c919193694.tar.gz |
Merge pull request #3628 from pks-t/pks/coverity-fixes
Coverity fixes
-rw-r--r-- | include/git2/sys/index.h | 2 | ||||
-rw-r--r-- | script/user_nodefs.h | 7 | ||||
-rw-r--r-- | src/common.h | 5 | ||||
-rw-r--r-- | src/crlf.c | 2 | ||||
-rw-r--r-- | src/diff_print.c | 6 | ||||
-rw-r--r-- | src/diff_tform.c | 27 | ||||
-rw-r--r-- | src/index.c | 24 | ||||
-rw-r--r-- | src/openssl_stream.c | 14 | ||||
-rw-r--r-- | src/pack-objects.c | 12 | ||||
-rw-r--r-- | src/path.c | 3 | ||||
-rw-r--r-- | src/rebase.c | 22 | ||||
-rw-r--r-- | src/refdb_fs.c | 3 | ||||
-rw-r--r-- | src/refspec.c | 8 | ||||
-rw-r--r-- | src/remote.c | 4 | ||||
-rw-r--r-- | src/revwalk.c | 3 | ||||
-rw-r--r-- | src/transports/smart_pkt.c | 25 | ||||
-rw-r--r-- | src/xdiff/xmerge.c | 2 |
17 files changed, 107 insertions, 62 deletions
diff --git a/include/git2/sys/index.h b/include/git2/sys/index.h index 29a99f798..2e2b87e68 100644 --- a/include/git2/sys/index.h +++ b/include/git2/sys/index.h @@ -25,7 +25,7 @@ typedef struct git_index_name_entry { /** Representation of a resolve undo entry in the index. */ typedef struct git_index_reuc_entry { - unsigned int mode[3]; + uint32_t mode[3]; git_oid oid[3]; char *path; } git_index_reuc_entry; diff --git a/script/user_nodefs.h b/script/user_nodefs.h index 3d25d92ec..3c06a706d 100644 --- a/script/user_nodefs.h +++ b/script/user_nodefs.h @@ -6,6 +6,7 @@ */ #nodef GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { __coverity_panic__(); } +#nodef GITERR_CHECK_ALLOC_BUF(buf) if (buf == NULL || git_buf_oom(buf)) { __coverity_panic__(); } #nodef GITERR_CHECK_ALLOC_ADD(out, one, two) \ if (GIT_ADD_SIZET_OVERFLOW(out, one, two)) { __coverity_panic__(); } @@ -25,3 +26,9 @@ #nodef GITERR_CHECK_VERSION(S,V,N) if (giterr__check_version(S,V,N) < 0) { __coverity_panic__(); } #nodef LOOKS_LIKE_DRIVE_PREFIX(S) (strlen(S) >= 2 && git__isalpha((S)[0]) && (S)[1] == ':') + +#nodef git_vector_foreach(v, iter, elem) \ + for ((iter) = 0; (v)->contents != NULL && (iter) < (v)->length && ((elem) = (v)->contents[(iter)], 1); (iter)++ ) + +#nodef git_vector_rforeach(v, iter, elem) \ + for ((iter) = (v)->length - 1; (v)->contents != NULL && (iter) < SIZE_MAX && ((elem) = (v)->contents[(iter)], 1); (iter)-- ) diff --git a/src/common.h b/src/common.h index bc4bdd856..9abd605cb 100644 --- a/src/common.h +++ b/src/common.h @@ -90,6 +90,11 @@ #define GITERR_CHECK_ALLOC(ptr) if (ptr == NULL) { return -1; } /** + * Check a buffer allocation result, returning -1 if it failed. + */ +#define GITERR_CHECK_ALLOC_BUF(buf) if ((void *)(buf) == NULL || git_buf_oom(buf)) { return -1; } + +/** * Check a return value and propagate result if non-zero. */ #define GITERR_CHECK_ERROR(code) \ diff --git a/src/crlf.c b/src/crlf.c index f391137c1..5d7510ac7 100644 --- a/src/crlf.c +++ b/src/crlf.c @@ -346,7 +346,7 @@ static int crlf_apply( /* initialize payload in case `check` was bypassed */ if (!*payload) { int error = crlf_check(self, payload, src, NULL); - if (error < 0 && error != GIT_PASSTHROUGH) + if (error < 0) return error; } diff --git a/src/diff_print.c b/src/diff_print.c index bc2d6fab0..dae9e341d 100644 --- a/src/diff_print.c +++ b/src/diff_print.c @@ -92,7 +92,11 @@ static int diff_print_info_init_frompatch( git_diff_line_cb cb, void *payload) { - git_repository *repo = patch && patch->diff ? patch->diff->repo : NULL; + git_repository *repo; + + assert(patch); + + repo = patch->diff ? patch->diff->repo : NULL; memset(pi, 0, sizeof(diff_print_info)); diff --git a/src/diff_tform.c b/src/diff_tform.c index 7cff34159..8577f06b8 100644 --- a/src/diff_tform.c +++ b/src/diff_tform.c @@ -261,18 +261,23 @@ static int normalize_find_opts( if (!given || (given->flags & GIT_DIFF_FIND_ALL) == GIT_DIFF_FIND_BY_CONFIG) { - char *rule = - git_config__get_string_force(cfg, "diff.renames", "true"); - int boolval; - - if (!git__parse_bool(&boolval, rule) && !boolval) - /* don't set FIND_RENAMES if bool value is false */; - else if (!strcasecmp(rule, "copies") || !strcasecmp(rule, "copy")) - opts->flags |= GIT_DIFF_FIND_RENAMES | GIT_DIFF_FIND_COPIES; - else - opts->flags |= GIT_DIFF_FIND_RENAMES; + if (diff->repo) { + char *rule = + git_config__get_string_force(cfg, "diff.renames", "true"); + int boolval; + + if (!git__parse_bool(&boolval, rule) && !boolval) + /* don't set FIND_RENAMES if bool value is false */; + else if (!strcasecmp(rule, "copies") || !strcasecmp(rule, "copy")) + opts->flags |= GIT_DIFF_FIND_RENAMES | GIT_DIFF_FIND_COPIES; + else + opts->flags |= GIT_DIFF_FIND_RENAMES; - git__free(rule); + git__free(rule); + } else { + /* set default flag */ + opts->flags |= GIT_DIFF_FIND_RENAMES; + } } /* some flags imply others */ diff --git a/src/index.c b/src/index.c index d0a0da2c5..483f7af7c 100644 --- a/src/index.c +++ b/src/index.c @@ -2135,11 +2135,11 @@ static int read_reuc(git_index *index, const char *buffer, size_t size) /* read 3 ASCII octal numbers for stage entries */ for (i = 0; i < 3; i++) { - int tmp; + int64_t tmp; - if (git__strtol32(&tmp, buffer, &endptr, 8) < 0 || + if (git__strtol64(&tmp, buffer, &endptr, 8) < 0 || !endptr || endptr == buffer || *endptr || - (unsigned)tmp > UINT_MAX) { + tmp < 0) { index_entry_reuc_free(lost); return index_error_invalid("reading reuc entry stage"); } @@ -2193,9 +2193,10 @@ static int read_conflict_names(git_index *index, const char *buffer, size_t size #define read_conflict_name(ptr) \ len = p_strnlen(buffer, size) + 1; \ - if (size < len) \ - return index_error_invalid("reading conflict name entries"); \ - \ + if (size < len) { \ + index_error_invalid("reading conflict name entries"); \ + goto out_err; \ + } \ if (len == 1) \ ptr = NULL; \ else { \ @@ -2216,7 +2217,16 @@ static int read_conflict_names(git_index *index, const char *buffer, size_t size read_conflict_name(conflict_name->theirs); if (git_vector_insert(&index->names, conflict_name) < 0) - return -1; + goto out_err; + + continue; + +out_err: + git__free(conflict_name->ancestor); + git__free(conflict_name->ours); + git__free(conflict_name->theirs); + git__free(conflict_name); + return -1; } #undef read_conflict_name diff --git a/src/openssl_stream.c b/src/openssl_stream.c index 15cabdfb8..97736b714 100644 --- a/src/openssl_stream.c +++ b/src/openssl_stream.c @@ -383,6 +383,8 @@ static int verify_server_cert(SSL *ssl, const char *host) GITERR_CHECK_ALLOC(peer_cn); memcpy(peer_cn, ASN1_STRING_data(str), size); peer_cn[size] = '\0'; + } else { + goto cert_fail_name; } } else { int size = ASN1_STRING_to_UTF8(&peer_cn, str); @@ -545,6 +547,7 @@ int git_openssl_stream_new(git_stream **out, const char *host, const char *port) st = git__calloc(1, sizeof(openssl_stream)); GITERR_CHECK_ALLOC(st); + st->io = NULL; #ifdef GIT_CURL error = git_curl_stream_new(&st->io, host, port); #else @@ -552,12 +555,13 @@ int git_openssl_stream_new(git_stream **out, const char *host, const char *port) #endif if (error < 0) - return error; + goto out_err; st->ssl = SSL_new(git__ssl_ctx); if (st->ssl == NULL) { giterr_set(GITERR_SSL, "failed to create ssl object"); - return -1; + error = -1; + goto out_err; } st->host = git__strdup(host); @@ -576,6 +580,12 @@ int git_openssl_stream_new(git_stream **out, const char *host, const char *port) *out = (git_stream *) st; return 0; + +out_err: + git_stream_free(st->io); + git__free(st); + + return error; } #else diff --git a/src/pack-objects.c b/src/pack-objects.c index 0afa28e62..46fe8f3db 100644 --- a/src/pack-objects.c +++ b/src/pack-objects.c @@ -629,10 +629,8 @@ static int write_pack(git_packbuilder *pb, int error = 0; write_order = compute_write_order(pb); - if (write_order == NULL) { - error = -1; - goto done; - } + if (write_order == NULL) + return -1; /* Write pack header */ ph.hdr_signature = htonl(PACK_SIGNATURE); @@ -850,9 +848,11 @@ static int try_delta(git_packbuilder *pb, struct unpacked *trg, git_packbuilder__cache_unlock(pb); - if (overflow || - !(trg_object->delta_data = git__realloc(delta_buf, delta_size))) + if (overflow) return -1; + + trg_object->delta_data = git__realloc(delta_buf, delta_size); + GITERR_CHECK_ALLOC(trg_object->delta_data); } else { /* create delta when writing the pack */ git_packbuilder__cache_unlock(pb); diff --git a/src/path.c b/src/path.c index 852ef576a..1fd14fcb9 100644 --- a/src/path.c +++ b/src/path.c @@ -705,8 +705,7 @@ int git_path_resolve_relative(git_buf *path, size_t ceiling) char *base, *to, *from, *next; size_t len; - if (!path || git_buf_oom(path)) - return -1; + GITERR_CHECK_ALLOC_BUF(path); if (ceiling > path->size) ceiling = path->size; diff --git a/src/rebase.c b/src/rebase.c index b9d1d4fc5..bcad9b7cd 100644 --- a/src/rebase.c +++ b/src/rebase.c @@ -257,12 +257,12 @@ done: return error; } -static git_rebase *rebase_alloc(const git_rebase_options *rebase_opts) +static int rebase_alloc(git_rebase **out, const git_rebase_options *rebase_opts) { git_rebase *rebase = git__calloc(1, sizeof(git_rebase)); + GITERR_CHECK_ALLOC(rebase); - if (!rebase) - return NULL; + *out = NULL; if (rebase_opts) memcpy(&rebase->options, rebase_opts, sizeof(git_rebase_options)); @@ -270,14 +270,16 @@ static git_rebase *rebase_alloc(const git_rebase_options *rebase_opts) git_rebase_init_options(&rebase->options, GIT_REBASE_OPTIONS_VERSION); if (rebase_opts && rebase_opts->rewrite_notes_ref) { - if ((rebase->options.rewrite_notes_ref = git__strdup(rebase_opts->rewrite_notes_ref)) == NULL) - return NULL; + rebase->options.rewrite_notes_ref = git__strdup(rebase_opts->rewrite_notes_ref); + GITERR_CHECK_ALLOC(rebase->options.rewrite_notes_ref); } if ((rebase->options.checkout_options.checkout_strategy & (GIT_CHECKOUT_SAFE | GIT_CHECKOUT_FORCE)) == 0) rebase->options.checkout_options.checkout_strategy = GIT_CHECKOUT_SAFE; - return rebase; + *out = rebase; + + return 0; } static int rebase_check_versions(const git_rebase_options *given_opts) @@ -305,8 +307,8 @@ int git_rebase_open( if ((error = rebase_check_versions(given_opts)) < 0) return error; - rebase = rebase_alloc(given_opts); - GITERR_CHECK_ALLOC(rebase); + if (rebase_alloc(&rebase, given_opts) < 0) + return -1; rebase->repo = repo; @@ -708,8 +710,8 @@ int git_rebase_init( branch = head_branch; } - rebase = rebase_alloc(given_opts); - GITERR_CHECK_ALLOC(rebase); + if (rebase_alloc(&rebase, given_opts) < 0) + return -1; rebase->repo = repo; rebase->inmemory = inmemory; diff --git a/src/refdb_fs.c b/src/refdb_fs.c index 1348c67a1..f6ed7201a 100644 --- a/src/refdb_fs.c +++ b/src/refdb_fs.c @@ -1512,8 +1512,7 @@ static int reflog_parse(git_reflog *log, const char *buf, size_t buf_size) #undef seek_forward fail: - if (entry) - git_reflog_entry__free(entry); + git_reflog_entry__free(entry); return -1; } diff --git a/src/refspec.c b/src/refspec.c index f92a6d2b6..debde8692 100644 --- a/src/refspec.c +++ b/src/refspec.c @@ -323,8 +323,8 @@ int git_refspec__dwim_one(git_vector *out, git_refspec *spec, git_vector *refs) if (git__prefixcmp(spec->src, GIT_REFS_DIR)) { for (j = 0; formatters[j]; j++) { git_buf_clear(&buf); - if (git_buf_printf(&buf, formatters[j], spec->src) < 0) - return -1; + git_buf_printf(&buf, formatters[j], spec->src); + GITERR_CHECK_ALLOC_BUF(&buf); key.name = (char *) git_buf_cstr(&buf); if (!git_vector_search(&pos, refs, &key)) { @@ -348,8 +348,8 @@ int git_refspec__dwim_one(git_vector *out, git_refspec *spec, git_vector *refs) git_buf_puts(&buf, GIT_REFS_HEADS_DIR); } - if (git_buf_puts(&buf, spec->dst) < 0) - return -1; + git_buf_puts(&buf, spec->dst); + GITERR_CHECK_ALLOC_BUF(&buf); cur->dst = git_buf_detach(&buf); } diff --git a/src/remote.c b/src/remote.c index 2f8ffcb37..8b7203ee2 100644 --- a/src/remote.c +++ b/src/remote.c @@ -208,8 +208,8 @@ static int create_internal(git_remote **out, git_repository *repo, const char *n remote->repo = repo; - if (git_vector_init(&remote->refs, 32, NULL) < 0 || - canonicalize_url(&canonical_url, url) < 0) + if ((error = git_vector_init(&remote->refs, 32, NULL)) < 0 || + (error = canonicalize_url(&canonical_url, url)) < 0) goto on_error; remote->url = apply_insteadof(repo->_config, canonical_url.ptr, GIT_DIRECTION_FETCH); diff --git a/src/revwalk.c b/src/revwalk.c index 89279ed1f..4815a1089 100644 --- a/src/revwalk.c +++ b/src/revwalk.c @@ -223,8 +223,7 @@ static int push_glob(git_revwalk *walk, const char *glob, int hide) git_buf_joinpath(&buf, GIT_REFS_DIR, glob); else git_buf_puts(&buf, glob); - if (git_buf_oom(&buf)) - return -1; + GITERR_CHECK_ALLOC_BUF(&buf); /* If no '?', '*' or '[' exist, we append '/ *' to the glob */ wildcard = strcspn(glob, "?*["); diff --git a/src/transports/smart_pkt.c b/src/transports/smart_pkt.c index 870f08497..2ea57bb64 100644 --- a/src/transports/smart_pkt.c +++ b/src/transports/smart_pkt.c @@ -296,13 +296,12 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) pkt = git__malloc(sizeof(*pkt)); GITERR_CHECK_ALLOC(pkt); + pkt->ref = NULL; pkt->type = GIT_PKT_NG; line += 3; /* skip "ng " */ - if (!(ptr = strchr(line, ' '))) { - giterr_set(GITERR_NET, "Invalid packet line"); - return -1; - } + if (!(ptr = strchr(line, ' '))) + goto out_err; len = ptr - line; GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); @@ -313,12 +312,8 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) pkt->ref[len] = '\0'; line = ptr + 1; - if (!(ptr = strchr(line, '\n'))) { - giterr_set(GITERR_NET, "Invalid packet line"); - git__free(pkt->ref); - git__free(pkt); - return -1; - } + if (!(ptr = strchr(line, '\n'))) + goto out_err; len = ptr - line; GITERR_CHECK_ALLOC_ADD(&alloclen, len, 1); @@ -330,6 +325,12 @@ static int ng_pkt(git_pkt **out, const char *line, size_t len) *out = (git_pkt *)pkt; return 0; + +out_err: + giterr_set(GITERR_NET, "Invalid packet line"); + git__free(pkt->ref); + git__free(pkt); + return -1; } static int unpack_pkt(git_pkt **out, const char *line, size_t len) @@ -543,7 +544,9 @@ static int buffer_want_with_caps(const git_remote_head *head, transport_smart_ca "%04xwant %s %s\n", (unsigned int)len, oid, git_buf_cstr(&str)); git_buf_free(&str); - return git_buf_oom(buf); + GITERR_CHECK_ALLOC_BUF(buf); + + return 0; } /* diff --git a/src/xdiff/xmerge.c b/src/xdiff/xmerge.c index 7b7e0e2d3..7928d1418 100644 --- a/src/xdiff/xmerge.c +++ b/src/xdiff/xmerge.c @@ -646,6 +646,8 @@ int xdl_merge(mmfile_t *orig, mmfile_t *mf1, mmfile_t *mf2, if (xdl_change_compact(&xe2.xdf1, &xe2.xdf2, xpp->flags) < 0 || xdl_change_compact(&xe2.xdf2, &xe2.xdf1, xpp->flags) < 0 || xdl_build_script(&xe2, &xscr2) < 0) { + xdl_free_script(xscr1); + xdl_free_env(&xe1); xdl_free_env(&xe2); return -1; } |