Commit 2a0d0bd19b5d13e2ab7f3780e094404828cbb9a7

Edward Thomson 2022-03-02T19:53:14

Merge pull request #6238 from libgit2/ethomson/coverity Some minor fixes for issues discovered by coverity

diff --git a/src/cli/opt_usage.c b/src/cli/opt_usage.c
index 6e5d600..478b416 100644
--- a/src/cli/opt_usage.c
+++ b/src/cli/opt_usage.c
@@ -150,7 +150,7 @@ int cli_opt_help_fprint(
 {
 	git_str help = GIT_BUF_INIT;
 	const cli_opt_spec *spec;
-	int error;
+	int error = 0;
 
 	/* Display required arguments first */
 	for (spec = specs; spec->type; ++spec) {
diff --git a/src/libgit2/object.c b/src/libgit2/object.c
index 7bc256f..b828f88 100644
--- a/src/libgit2/object.c
+++ b/src/libgit2/object.c
@@ -250,6 +250,7 @@ int git_object_lookup_prefix(
 	if (error < 0)
 		return error;
 
+	GIT_ASSERT(odb_obj);
 	error = git_object__from_odb_object(object_out, repo, odb_obj, type);
 
 	git_odb_object_free(odb_obj);
diff --git a/src/libgit2/refdb_fs.c b/src/libgit2/refdb_fs.c
index 95bda94..ef85016 100644
--- a/src/libgit2/refdb_fs.c
+++ b/src/libgit2/refdb_fs.c
@@ -1361,7 +1361,11 @@ 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);
-		GIT_ASSERT(ref);
+
+		GIT_ASSERT_WITH_CLEANUP(ref, {
+			error = -1;
+			goto fail;
+		});
 
 		if ((error = packed_find_peel(backend, ref)) < 0)
 			goto fail;
diff --git a/src/libgit2/transports/httpclient.c b/src/libgit2/transports/httpclient.c
index 75782da..f07923e 100644
--- a/src/libgit2/transports/httpclient.c
+++ b/src/libgit2/transports/httpclient.c
@@ -395,7 +395,7 @@ static int on_body(http_parser *parser, const char *buf, size_t len)
 	size_t max_len;
 
 	/* Saw data when we expected not to (eg, in consume_response_body) */
-	if (ctx->output_buf == NULL && ctx->output_size == 0) {
+	if (ctx->output_buf == NULL || ctx->output_size == 0) {
 		ctx->parse_status = PARSE_STATUS_NO_OUTPUT;
 		return 0;
 	}
diff --git a/src/util/assert_safe.h b/src/util/assert_safe.h
index 8c26110..cc0bac5 100644
--- a/src/util/assert_safe.h
+++ b/src/util/assert_safe.h
@@ -24,6 +24,8 @@
 
 # define GIT_ASSERT_WITH_RETVAL(expr, fail) assert(expr)
 # define GIT_ASSERT_ARG_WITH_RETVAL(expr, fail) assert(expr)
+
+# define GIT_ASSERT_WITH_CLEANUP(expr, cleanup) assert(expr)
 #else
 
 /** Internal consistency check to stop the function. */
@@ -53,6 +55,20 @@
 		} \
 	} while(0)
 
+/**
+ * Go to to the given label on assertion failures; useful when you have
+ * taken a lock or otherwise need to release a resource.
+ */
+# define GIT_ASSERT_WITH_CLEANUP(expr, cleanup) \
+	GIT_ASSERT__WITH_CLEANUP(expr, GIT_ERROR_INTERNAL, "unrecoverable internal error", cleanup)
+
+# define GIT_ASSERT__WITH_CLEANUP(expr, code, msg, cleanup) do { \
+		if (!(expr)) { \
+			git_error_set(code, "%s: '%s'", msg, #expr); \
+			cleanup; \
+		} \
+	} while(0)
+
 #endif /* GIT_ASSERT_HARD */
 
 #endif
diff --git a/src/util/fs_path.c b/src/util/fs_path.c
index 920c390..6584242 100644
--- a/src/util/fs_path.c
+++ b/src/util/fs_path.c
@@ -109,7 +109,7 @@ int git_fs_path_basename_r(git_str *buffer, const char *path)
 	/* Empty or NULL string gets treated as "." */
 	if (path == NULL || *path == '\0') {
 		startp = ".";
-		len		= 1;
+		len = 1;
 		goto Exit;
 	}
 
@@ -121,7 +121,7 @@ int git_fs_path_basename_r(git_str *buffer, const char *path)
 	/* All slashes becomes "/" */
 	if (endp == path && *endp == '/') {
 		startp = "/";
-		len	= 1;
+		len = 1;
 		goto Exit;
 	}
 
@@ -193,8 +193,7 @@ int git_fs_path_dirname_r(git_str *buffer, const char *path)
 
 	if (endp - path + 1 > INT_MAX) {
 		git_error_set(GIT_ERROR_INVALID, "path too long");
-		len = -1;
-		goto Exit;
+		return -1;
 	}
 
 	if ((len = win32_prefix_length(path, (int)(endp - path + 1))) > 0) {
@@ -219,8 +218,7 @@ int git_fs_path_dirname_r(git_str *buffer, const char *path)
 
 	if (endp - path + 1 > INT_MAX) {
 		git_error_set(GIT_ERROR_INVALID, "path too long");
-		len = -1;
-		goto Exit;
+		return -1;
 	}
 
 	if ((len = win32_prefix_length(path, (int)(endp - path + 1))) > 0) {
diff --git a/tests/util/assert.c b/tests/util/assert.c
index 26c4297..3babc47 100644
--- a/tests/util/assert.c
+++ b/tests/util/assert.c
@@ -36,6 +36,21 @@ static const char *bad_returns_string(void)
 	return hello_world;
 }
 
+static int has_cleanup(void)
+{
+	int error = 42;
+
+	GIT_ASSERT_WITH_CLEANUP(1 + 1 == 3, {
+		error = 99;
+		goto foobar;
+	});
+
+	return 0;
+
+foobar:
+	return error;
+}
+
 void test_assert__argument(void)
 {
 	cl_git_fail(dummy_fn(NULL));
@@ -92,3 +107,11 @@ void test_assert__internal(void)
 	cl_assert_equal_i(GIT_ERROR_INTERNAL, git_error_last()->klass);
 	cl_assert_equal_s("unrecoverable internal error: '1 + 1 == 3'", git_error_last()->message);
 }
+
+void test_assert__with_cleanup(void)
+{
+	cl_git_fail_with(99, has_cleanup());
+	cl_assert(git_error_last());
+	cl_assert_equal_i(GIT_ERROR_INTERNAL, git_error_last()->klass);
+	cl_assert_equal_s("unrecoverable internal error: '1 + 1 == 3'", git_error_last()->message);
+}