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

Coverity fixes

9 files changed, 132 insertions, 93 deletions

diff --git a/src/cache.c b/src/cache.c
index 32ba993b0..af42b3959 100644
--- a/src/cache.c
+++ b/src/cache.c
@@ -208,10 +208,14 @@ static void *cache_store(git_cache *cache, git_cached_obj *entry)
 			entry = stored_entry;
 		} else if (stored_entry->flags == GIT_CACHE_STORE_RAW &&
 			   entry->flags == GIT_CACHE_STORE_PARSED) {
-			git_cached_obj_decref(stored_entry);
-			git_cached_obj_incref(entry);
-
-			git_oidmap_set(cache->map, &entry->oid, entry);
+			if (git_oidmap_set(cache->map, &entry->oid, entry) == 0) {
+				git_cached_obj_decref(stored_entry);
+				git_cached_obj_incref(entry);
+			} else {
+				git_cached_obj_decref(entry);
+				git_cached_obj_incref(stored_entry);
+				entry = stored_entry;
+			}
 		} else {
 			/* NO OP */
 		}
diff --git a/src/indexer.c b/src/indexer.c
index 717549fa2..68fdd85c5 100644
--- a/src/indexer.c
+++ b/src/indexer.c
@@ -150,11 +150,11 @@ int git_indexer_new(
 	idx->progress_cb = opts.progress_cb;
 	idx->progress_payload = opts.progress_cb_payload;
 	idx->mode = mode ? mode : GIT_PACK_FILE_MODE;
-	git_hash_ctx_init(&idx->hash_ctx);
-	git_hash_ctx_init(&idx->trailer);
 	git_buf_init(&idx->entry_data, 0);
 
-	if ((error = git_oidmap_new(&idx->expected_oids)) < 0)
+	if ((error = git_hash_ctx_init(&idx->hash_ctx)) < 0 ||
+	    (error = git_hash_ctx_init(&idx->trailer)) < 0 ||
+	    (error = git_oidmap_new(&idx->expected_oids)) < 0)
 		goto cleanup;
 
 	idx->do_verify = opts.verify;
diff --git a/src/notes.c b/src/notes.c
index 4633a16ea..68d2ae9ec 100644
--- a/src/notes.c
+++ b/src/notes.c
@@ -808,8 +808,11 @@ int git_note_next(
 
 	git_oid_cpy(note_id, &item->id);
 
-	if (!(error = process_entry_path(item->path, annotated_id)))
-		git_iterator_advance(NULL, it);
+	if ((error = process_entry_path(item->path, annotated_id)) < 0)
+		return error;
 
-	return error;
+	if ((error = git_iterator_advance(NULL, it)) < 0 && error != GIT_ITEROVER)
+		return error;
+
+	return 0;
 }
diff --git a/src/pack-objects.c b/src/pack-objects.c
index bdd5171a8..49b4e4772 100644
--- a/src/pack-objects.c
+++ b/src/pack-objects.c
@@ -374,7 +374,9 @@ static int write_object(
 		GIT_ERROR_CHECK_ALLOC(zbuf);
 
 		git_zstream_reset(&pb->zstream);
-		git_zstream_set_input(&pb->zstream, data, data_len);
+
+		if ((error = git_zstream_set_input(&pb->zstream, data, data_len)) < 0)
+			goto done;
 
 		while (!git_zstream_done(&pb->zstream)) {
 			if ((error = git_zstream_get_output(zbuf, &zbuf_len, &pb->zstream)) < 0 ||
diff --git a/src/push.c b/src/push.c
index 67ebcfb3e..34867c2e4 100644
--- a/src/push.c
+++ b/src/push.c
@@ -349,8 +349,9 @@ static int queue_objects(git_push *push)
 		if (git_oid_is_zero(&head->oid))
 			continue;
 
-		/* TODO */
-		git_revwalk_hide(rw, &head->oid);
+		if ((error = git_revwalk_hide(rw, &head->oid)) < 0 &&
+		    error != GIT_ENOTFOUND && error != GIT_EINVALIDSPEC && error != GIT_EPEEL)
+			goto on_error;
 	}
 
 	error = git_packbuilder_insert_walk(push->pb, rw);
diff --git a/src/repository.c b/src/repository.c
index 2469e13e6..fe0d696c6 100644
--- a/src/repository.c
+++ b/src/repository.c
@@ -189,19 +189,25 @@ void git_repository_free(git_repository *repo)
  *
  * Open a repository object from its path
  */
-static bool valid_repository_path(git_buf *repository_path, git_buf *common_path)
+static int is_valid_repository_path(bool *out, git_buf *repository_path, git_buf *common_path)
 {
+	int error;
+
+	*out = false;
+
 	/* Check if we have a separate commondir (e.g. we have a
 	 * worktree) */
 	if (git_path_contains_file(repository_path, GIT_COMMONDIR_FILE)) {
 		git_buf common_link  = GIT_BUF_INIT;
-		git_buf_joinpath(&common_link, repository_path->ptr, GIT_COMMONDIR_FILE);
 
-		git_futils_readbuffer(&common_link, common_link.ptr);
-		git_buf_rtrim(&common_link);
+		if ((error = git_buf_joinpath(&common_link, repository_path->ptr, GIT_COMMONDIR_FILE)) < 0 ||
+		    (error = git_futils_readbuffer(&common_link, common_link.ptr)) < 0)
+			return error;
 
+		git_buf_rtrim(&common_link);
 		if (git_path_is_relative(common_link.ptr)) {
-			git_buf_joinpath(common_path, repository_path->ptr, common_link.ptr);
+			if ((error = git_buf_joinpath(common_path, repository_path->ptr, common_link.ptr)) < 0)
+				return error;
 		} else {
 			git_buf_swap(common_path, &common_link);
 		}
@@ -209,24 +215,26 @@ static bool valid_repository_path(git_buf *repository_path, git_buf *common_path
 		git_buf_dispose(&common_link);
 	}
 	else {
-		git_buf_set(common_path, repository_path->ptr, repository_path->size);
+		if ((error = git_buf_set(common_path, repository_path->ptr, repository_path->size)) < 0)
+			return error;
 	}
 
 	/* Make sure the commondir path always has a trailing * slash */
 	if (git_buf_rfind(common_path, '/') != (ssize_t)common_path->size - 1)
-		git_buf_putc(common_path, '/');
+		if ((error = git_buf_putc(common_path, '/')) < 0)
+			return error;
 
 	/* Ensure HEAD file exists */
 	if (git_path_contains_file(repository_path, GIT_HEAD_FILE) == false)
-		return false;
-
+		return 0;
 	/* Check files in common dir */
 	if (git_path_contains_dir(common_path, GIT_OBJECTS_DIR) == false)
-		return false;
+		return 0;
 	if (git_path_contains_dir(common_path, GIT_REFS_DIR) == false)
-		return false;
+		return 0;
 
-	return true;
+	*out = true;
+	return 0;
 }
 
 static git_repository *repository_alloc(void)
@@ -441,15 +449,15 @@ static int find_repo(
 	uint32_t flags,
 	const char *ceiling_dirs)
 {
-	int error;
 	git_buf path = GIT_BUF_INIT;
 	git_buf repo_link = GIT_BUF_INIT;
 	git_buf common_link = GIT_BUF_INIT;
 	struct stat st;
 	dev_t initial_device = 0;
 	int min_iterations;
-	bool in_dot_git;
+	bool in_dot_git, is_valid;
 	size_t ceiling_offset = 0;
+	int error;
 
 	git_buf_clear(gitdir_path);
 
@@ -475,9 +483,8 @@ static int find_repo(
 	for (;;) {
 		if (!(flags & GIT_REPOSITORY_OPEN_NO_DOTGIT)) {
 			if (!in_dot_git) {
-				error = git_buf_joinpath(&path, path.ptr, DOT_GIT);
-				if (error < 0)
-					break;
+				if ((error = git_buf_joinpath(&path, path.ptr, DOT_GIT)) < 0)
+					goto out;
 			}
 			in_dot_git = !in_dot_git;
 		}
@@ -491,28 +498,33 @@ static int find_repo(
 				break;
 
 			if (S_ISDIR(st.st_mode)) {
-				if (valid_repository_path(&path, &common_link)) {
-					git_path_to_dir(&path);
-					git_buf_set(gitdir_path, path.ptr, path.size);
+				if ((error = is_valid_repository_path(&is_valid, &path, &common_link)) < 0)
+					goto out;
+
+				if (is_valid) {
+					if ((error = git_path_to_dir(&path)) < 0 ||
+					    (error = git_buf_set(gitdir_path, path.ptr, path.size)) < 0)
+						goto out;
 
 					if (gitlink_path)
-						git_buf_attach(gitlink_path,
-							git_worktree__read_link(path.ptr, GIT_GITDIR_FILE), 0);
+						if ((error = git_buf_attach(gitlink_path, git_worktree__read_link(path.ptr, GIT_GITDIR_FILE), 0)) < 0)
+							goto out;
 					if (commondir_path)
 						git_buf_swap(&common_link, commondir_path);
 
 					break;
 				}
-			}
-			else if (S_ISREG(st.st_mode) && git__suffixcmp(path.ptr, "/" DOT_GIT) == 0) {
-				error = read_gitfile(&repo_link, path.ptr);
-				if (error < 0)
-					break;
-				if (valid_repository_path(&repo_link, &common_link)) {
+			} else if (S_ISREG(st.st_mode) && git__suffixcmp(path.ptr, "/" DOT_GIT) == 0) {
+				if ((error = read_gitfile(&repo_link, path.ptr)) < 0 ||
+				    (error = is_valid_repository_path(&is_valid, &repo_link, &common_link)) < 0)
+					goto out;
+
+				if (is_valid) {
 					git_buf_swap(gitdir_path, &repo_link);
 
 					if (gitlink_path)
-						error = git_buf_put(gitlink_path, path.ptr, path.size);
+						if ((error = git_buf_put(gitlink_path, path.ptr, path.size)) < 0)
+							goto out;
 					if (commondir_path)
 						git_buf_swap(&common_link, commondir_path);
 				}
@@ -523,10 +535,8 @@ static int find_repo(
 		/* Move up one directory. If we're in_dot_git, we'll search the
 		 * parent itself next. If we're !in_dot_git, we'll search .git
 		 * in the parent directory next (added at the top of the loop). */
-		if (git_path_dirname_r(&path, path.ptr) < 0) {
-			error = -1;
-			break;
-		}
+		if ((error = git_path_dirname_r(&path, path.ptr)) < 0)
+			goto out;
 
 		/* Once we've checked the directory (and .git if applicable),
 		 * find the ceiling for a search. */
@@ -534,31 +544,28 @@ static int find_repo(
 			ceiling_offset = find_ceiling_dir_offset(path.ptr, ceiling_dirs);
 
 		/* Check if we should stop searching here. */
-		if (min_iterations == 0
-		    && (path.ptr[ceiling_offset] == 0
-			|| (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))
+		if (min_iterations == 0 &&
+		    (path.ptr[ceiling_offset] == 0 || (flags & GIT_REPOSITORY_OPEN_NO_SEARCH)))
 			break;
 	}
 
-	if (!error && workdir_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
+	if (workdir_path && !(flags & GIT_REPOSITORY_OPEN_BARE)) {
 		if (!git_buf_len(gitdir_path))
 			git_buf_clear(workdir_path);
-		else {
-			git_path_dirname_r(workdir_path, path.ptr);
-			git_path_to_dir(workdir_path);
-		}
-		if (git_buf_oom(workdir_path))
-			return -1;
+		else if ((error = git_path_dirname_r(workdir_path, path.ptr)) < 0 ||
+			 (error = git_path_to_dir(workdir_path)) < 0)
+			goto out;
 	}
 
 	/* If we didn't find the repository, and we don't have any other error
 	 * to report, report that. */
-	if (!git_buf_len(gitdir_path) && !error) {
-		git_error_set(GIT_ERROR_REPOSITORY,
-			"could not find repository from '%s'", start_path);
+	if (!git_buf_len(gitdir_path)) {
+		git_error_set(GIT_ERROR_REPOSITORY, "could not find repository from '%s'", start_path);
 		error = GIT_ENOTFOUND;
+		goto out;
 	}
 
+out:
 	git_buf_dispose(&path);
 	git_buf_dispose(&repo_link);
 	git_buf_dispose(&common_link);
@@ -569,14 +576,16 @@ int git_repository_open_bare(
 	git_repository **repo_ptr,
 	const char *bare_path)
 {
-	int error;
 	git_buf path = GIT_BUF_INIT, common_path = GIT_BUF_INIT;
 	git_repository *repo = NULL;
+	bool is_valid;
+	int error;
 
-	if ((error = git_path_prettify_dir(&path, bare_path, NULL)) < 0)
+	if ((error = git_path_prettify_dir(&path, bare_path, NULL)) < 0 ||
+	    (error = is_valid_repository_path(&is_valid, &path, &common_path)) < 0)
 		return error;
 
-	if (!valid_repository_path(&path, &common_path)) {
+	if (!is_valid) {
 		git_buf_dispose(&path);
 		git_buf_dispose(&common_path);
 		git_error_set(GIT_ERROR_REPOSITORY, "path is not a repository: %s", bare_path);
@@ -2055,6 +2064,7 @@ int git_repository_init_ext(
 	git_buf repo_path = GIT_BUF_INIT, wd_path = GIT_BUF_INIT,
 		common_path = GIT_BUF_INIT, head_path = GIT_BUF_INIT;
 	const char *wd;
+	bool is_valid;
 	int error;
 
 	assert(out && given_repo && opts);
@@ -2066,7 +2076,10 @@ int git_repository_init_ext(
 
 	wd = (opts->flags & GIT_REPOSITORY_INIT_BARE) ? NULL : git_buf_cstr(&wd_path);
 
-	if (valid_repository_path(&repo_path, &common_path)) {
+	if ((error = is_valid_repository_path(&is_valid, &repo_path, &common_path)) < 0)
+		goto out;
+
+	if (is_valid) {
 		if ((opts->flags & GIT_REPOSITORY_INIT_NO_REINIT) != 0) {
 			git_error_set(GIT_ERROR_REPOSITORY,
 				"attempt to reinitialize '%s'", given_repo);
diff --git a/src/revwalk.c b/src/revwalk.c
index 4587b5acc..abbd65ac2 100644
--- a/src/revwalk.c
+++ b/src/revwalk.c
@@ -58,7 +58,7 @@ int git_revwalk__push_commit(git_revwalk *walk, const git_oid *oid, const git_re
 			return 0;
 
 		git_error_set(GIT_ERROR_INVALID, "object is not a committish");
-		return -1;
+		return error;
 	}
 	if (error < 0)
 		return error;
diff --git a/src/streams/openssl.c b/src/streams/openssl.c
index 458810c91..5b66352ca 100644
--- a/src/streams/openssl.c
+++ b/src/streams/openssl.c
@@ -156,7 +156,7 @@ static void openssl_locking_function(
 	lock = mode & CRYPTO_LOCK;
 
 	if (lock) {
-		git_mutex_lock(&openssl_locks[n]);
+		(void)git_mutex_lock(&openssl_locks[n]);
 	} else {
 		git_mutex_unlock(&openssl_locks[n]);
 	}
diff --git a/src/worktree.c b/src/worktree.c
index ef4ebfda8..e171afbb2 100644
--- a/src/worktree.c
+++ b/src/worktree.c
@@ -136,11 +136,11 @@ static int open_worktree_dir(git_worktree **out, const char *parent, const char
 		goto out;
 	}
 
-	if ((wt->name = git__strdup(name)) == NULL
-	    || (wt->commondir_path = git_worktree__read_link(dir, "commondir")) == NULL
-	    || (wt->gitlink_path = git_worktree__read_link(dir, "gitdir")) == NULL
-	    || (parent && (wt->parent_path = git__strdup(parent)) == NULL)
-	    || (wt->worktree_path = git_path_dirname(wt->gitlink_path)) == NULL) {
+	if ((wt->name = git__strdup(name)) == NULL ||
+	    (wt->commondir_path = git_worktree__read_link(dir, "commondir")) == NULL ||
+	    (wt->gitlink_path = git_worktree__read_link(dir, "gitdir")) == NULL ||
+	    (parent && (wt->parent_path = git__strdup(parent)) == NULL) ||
+	    (wt->worktree_path = git_path_dirname(wt->gitlink_path)) == NULL) {
 		error = -1;
 		goto out;
 	}
@@ -149,7 +149,10 @@ static int open_worktree_dir(git_worktree **out, const char *parent, const char
 		goto out;
 	wt->gitdir_path = git_buf_detach(&gitdir);
 
-	wt->locked = !!git_worktree_is_locked(NULL, wt);
+	if ((error = git_worktree_is_locked(NULL, wt)) < 0)
+		goto out;
+	wt->locked = !!error;
+	error = 0;
 
 	*out = wt;
 
@@ -403,20 +406,24 @@ out:
 int git_worktree_lock(git_worktree *wt, const char *reason)
 {
 	git_buf buf = GIT_BUF_INIT, path = GIT_BUF_INIT;
-	int err;
+	int error;
 
 	assert(wt);
 
-	if ((err = git_worktree_is_locked(NULL, wt)) < 0)
+	if ((error = git_worktree_is_locked(NULL, wt)) < 0)
+		goto out;
+	if (error) {
+		error = GIT_ELOCKED;
 		goto out;
+	}
 
-	if ((err = git_buf_joinpath(&path, wt->gitdir_path, "locked")) < 0)
+	if ((error = git_buf_joinpath(&path, wt->gitdir_path, "locked")) < 0)
 		goto out;
 
 	if (reason)
 		git_buf_attach_notowned(&buf, reason, strlen(reason));
 
-	if ((err = git_futils_writebuffer(&buf, path.ptr, O_CREAT|O_EXCL|O_WRONLY, 0644)) < 0)
+	if ((error = git_futils_writebuffer(&buf, path.ptr, O_CREAT|O_EXCL|O_WRONLY, 0644)) < 0)
 		goto out;
 
 	wt->locked = 1;
@@ -424,16 +431,19 @@ int git_worktree_lock(git_worktree *wt, const char *reason)
 out:
 	git_buf_dispose(&path);
 
-	return err;
+	return error;
 }
 
 int git_worktree_unlock(git_worktree *wt)
 {
 	git_buf path = GIT_BUF_INIT;
+	int error;
 
 	assert(wt);
 
-	if (!git_worktree_is_locked(NULL, wt))
+	if ((error = git_worktree_is_locked(NULL, wt)) < 0)
+		return error;
+	if (!error)
 		return 1;
 
 	if (git_buf_joinpath(&path, wt->gitdir_path, "locked") < 0)
@@ -454,22 +464,25 @@ int git_worktree_unlock(git_worktree *wt)
 int git_worktree_is_locked(git_buf *reason, const git_worktree *wt)
 {
 	git_buf path = GIT_BUF_INIT;
-	int ret;
+	int error, locked;
 
 	assert(wt);
 
 	if (reason)
 		git_buf_clear(reason);
 
-	if ((ret = git_buf_joinpath(&path, wt->gitdir_path, "locked")) < 0)
+	if ((error = git_buf_joinpath(&path, wt->gitdir_path, "locked")) < 0)
+		goto out;
+	locked = git_path_exists(path.ptr);
+	if (locked && reason &&
+	    (error = git_futils_readbuffer(reason, path.ptr)) < 0)
 		goto out;
-	if ((ret = git_path_exists(path.ptr)) && reason)
-		git_futils_readbuffer(reason, path.ptr);
 
+	error = locked;
 out:
 	git_buf_dispose(&path);
 
-	return ret;
+	return error;
 }
 
 const char *git_worktree_name(const git_worktree *wt)
@@ -502,7 +515,6 @@ int git_worktree_pruneinit_options(git_worktree_prune_options *opts,
 int git_worktree_is_prunable(git_worktree *wt,
 	git_worktree_prune_options *opts)
 {
-	git_buf reason = GIT_BUF_INIT;
 	git_worktree_prune_options popts = GIT_WORKTREE_PRUNE_OPTIONS_INIT;
 
 	GIT_ERROR_CHECK_VERSION(
@@ -512,20 +524,24 @@ int git_worktree_is_prunable(git_worktree *wt,
 	if (opts)
 		memcpy(&popts, opts, sizeof(popts));
 
-	if ((popts.flags & GIT_WORKTREE_PRUNE_LOCKED) == 0 &&
-		git_worktree_is_locked(&reason, wt))
-	{
-		if (!reason.size)
-			git_buf_attach_notowned(&reason, "no reason given", 15);
-		git_error_set(GIT_ERROR_WORKTREE, "not pruning locked working tree: '%s'", reason.ptr);
-		git_buf_dispose(&reason);
+	if ((popts.flags & GIT_WORKTREE_PRUNE_LOCKED) == 0) {
+		git_buf reason = GIT_BUF_INIT;
+		int error;
 
-		return 0;
+		if ((error = git_worktree_is_locked(&reason, wt)) < 0)
+			return error;
+
+		if (error) {
+			if (!reason.size)
+				git_buf_attach_notowned(&reason, "no reason given", 15);
+			git_error_set(GIT_ERROR_WORKTREE, "not pruning locked working tree: '%s'", reason.ptr);
+			git_buf_dispose(&reason);
+			return 0;
+		}
 	}
 
 	if ((popts.flags & GIT_WORKTREE_PRUNE_VALID) == 0 &&
-		git_worktree_validate(wt) == 0)
-	{
+	    git_worktree_validate(wt) == 0) {
 		git_error_set(GIT_ERROR_WORKTREE, "not pruning valid working tree");
 		return 0;
 	}