errors: never return a NULL error

Callers want to be able to simply call `git_error_last()->message`,
not have to worry about whether `git_error_last()` returns NULL or not.

include/git2/errors.h

@@ -118,10 +118,15 @@ typedef enum {
  * Return the last `git_error` object that was generated for the
  * current thread.
  *
- * The default behaviour of this function is to return NULL if no previous error has occurred.
- * However, libgit2's error strings are not cleared aggressively, so a prior
- * (unrelated) error may be returned. This can be avoided by only calling
- * this function if the prior call to a libgit2 API returned an error.
+ * This function will never return NULL.
+ *
+ * Callers should not rely on this to determine whether an error has
+ * occurred. For error checking, callers should examine the return
+ * codes of libgit2 functions.
+ *
+ * This call can only reliably report error messages when an error
+ * has occurred. (It may contain stale information if it is called
+ * after a different function that succeeds.)
  *
  * @return A git_error object.
  */

src/libgit2/errors.c

@@ -28,7 +28,12 @@ static git_error uninitialized_error = {
 
 static git_error tlsdata_error = {
 	"thread-local data initialization failure",
-	GIT_ERROR
+	GIT_ERROR_THREAD
+};
+
+static git_error no_error = {
+	"no error",
+	GIT_ERROR_NONE
 };
 
 static void set_error_from_buffer(int error_class)
@@ -184,6 +189,9 @@ const git_error *git_error_last(void)
 	if ((threadstate = git_threadstate_get()) == NULL)
 		return &tlsdata_error;
 
+	if (!threadstate->last_error)
+		return &no_error;
+
 	return threadstate->last_error;
 }
 

tests/libgit2/config/include.c

@@ -111,7 +111,7 @@ void test_config_include__missing(void)
 
 	git_error_clear();
 	cl_git_pass(git_config_open_ondisk(&cfg, "including"));
-	cl_assert(git_error_last() == NULL);
+	cl_assert_equal_i(GIT_ERROR_NONE, git_error_last()->klass);
 	cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar"));
 	cl_assert_equal_s("baz", buf.ptr);
 
@@ -126,7 +126,7 @@ void test_config_include__missing_homedir(void)
 
 	git_error_clear();
 	cl_git_pass(git_config_open_ondisk(&cfg, "including"));
-	cl_assert(git_error_last() == NULL);
+	cl_assert_equal_i(GIT_ERROR_NONE, git_error_last()->klass);
 	cl_git_pass(git_config_get_string_buf(&buf, cfg, "foo.bar"));
 	cl_assert_equal_s("baz", buf.ptr);
 

tests/libgit2/grafts/shallow.c

@@ -40,7 +40,7 @@ void test_grafts_shallow__clears_errors(void)
 {
 	g_repo = cl_git_sandbox_init("testrepo.git");
 	cl_assert_equal_i(0, git_repository_is_shallow(g_repo));
-	cl_assert_equal_p(NULL, git_error_last());
+	cl_assert_equal_i(GIT_ERROR_NONE, git_error_last()->klass);
 }
 
 void test_grafts_shallow__shallow_oids(void)

tests/libgit2/repo/shallow.c

@@ -35,5 +35,5 @@ void test_repo_shallow__clears_errors(void)
 {
 	g_repo = cl_git_sandbox_init("testrepo.git");
 	cl_assert_equal_i(0, git_repository_is_shallow(g_repo));
-	cl_assert_equal_p(NULL, git_error_last());
+	cl_assert_equal_i(GIT_ERROR_NONE, git_error_last()->klass);
 }

tests/util/assert.c

@@ -92,7 +92,8 @@ void test_assert__argument_with_void_return_type(void)
 
 	git_error_clear();
 	cl_assert_equal_p(foo, fn_returns_string(foo));
-	cl_assert_equal_p(NULL, git_error_last());
+	cl_assert_equal_i(GIT_ERROR_NONE, git_error_last()->klass);
+	cl_assert_equal_s("no error", git_error_last()->message);
 }
 
 void test_assert__internal(void)

tests/util/errors.c

@@ -5,7 +5,10 @@ void test_errors__public_api(void)
 	char *str_in_error;
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
+
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	git_error_set_oom();
 
@@ -23,7 +26,9 @@ void test_errors__public_api(void)
 	cl_assert(str_in_error != NULL);
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 }
 
 #include "common.h"
@@ -35,7 +40,9 @@ void test_errors__new_school(void)
 	char *str_in_error;
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	git_error_set_oom(); /* internal fn */
 
@@ -53,7 +60,9 @@ void test_errors__new_school(void)
 	cl_assert(str_in_error != NULL);
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	do {
 		struct stat st;
@@ -91,7 +100,9 @@ void test_errors__restore(void)
 	git_error_state err_state = {0};
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	cl_assert_equal_i(0, git_error_state_capture(&err_state, 0));
 
@@ -100,7 +111,9 @@ void test_errors__restore(void)
 	git_error_set(42, "Foo: %s", "bar");
 	cl_assert_equal_i(-1, git_error_state_capture(&err_state, -1));
 
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	git_error_set(99, "Bar: %s", "foo");
 
@@ -128,7 +141,9 @@ void test_errors__free_state(void)
 
 	git_error_state_restore(&err_state);
 
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 }
 
 void test_errors__restore_oom(void)
@@ -144,7 +159,9 @@ void test_errors__restore_oom(void)
 
 	cl_assert_equal_i(-1, git_error_state_capture(&err_state, -1));
 
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 	cl_assert_equal_i(GIT_ERROR_NOMEMORY, err_state.error_msg.klass);
 	cl_assert_equal_s("Out of memory", err_state.error_msg.message);
 
@@ -204,11 +221,15 @@ void test_errors__integer_overflow_sets_oom(void)
 
 	git_error_clear();
 	cl_assert(!GIT_ADD_SIZET_OVERFLOW(&out, SIZE_MAX-1, 1));
-	cl_assert_equal_p(NULL, git_error_last());
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	git_error_clear();
 	cl_assert(!GIT_ADD_SIZET_OVERFLOW(&out, 42, 69));
-	cl_assert_equal_p(NULL, git_error_last());
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp(git_error_last()->message, "no error") == 0);
 
 	git_error_clear();
 	cl_assert(GIT_ADD_SIZET_OVERFLOW(&out, SIZE_MAX, SIZE_MAX));