Merge pull request #3628 from pks-t/pks/coverity-fixes

Coverity fixes

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;

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)-- )

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) \

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;
 	}
 

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));
 

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 */

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

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

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);

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;

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;

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;
 }

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);
 	}

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);

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, "?*[");

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;
 }
 
 /*

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;
 	}