Merge pull request #6238 from libgit2/ethomson/coverity Some minor fixes for issues discovered by coverity
1 2 3 4 5 6 7 8 9 10 11 12 13 14 15 16 17 18 19 20 21 22 23 24 25 26 27 28 29 30 31 32 33 34 35 36 37 38 39 40 41 42 43 44 45 46 47 48 49 50 51 52 53 54 55 56 57 58 59 60 61 62 63 64 65 66 67 68 69 70 71 72 73 74 75 76 77 78 79 80 81 82 83 84 85 86 87 88 89 90 91 92 93 94 95 96 97 98 99 100 101 102 103 104 105 106 107 108 109 110 111 112 113 114 115 116 117 118 119 120 121 122 123 124 125 126 127 128 129 130 131 132 133 134 135 136 137 138 139 140 141 142 143 144 145 146 147 148 149 150 151 152 153 154 155 156 157 158 159 160 161 162 163 164 165 166 167 168 169
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);
+}