Merge pull request #6238 from libgit2/ethomson/coverity

Some minor fixes for issues discovered by coverity

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

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

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;

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

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

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

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