Merge pull request #6625 from libgit2/ethomson/error_updates

include/git2/errors.h

@@ -118,63 +118,22 @@ 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.
  *
- * @return A git_error object.
- */
-GIT_EXTERN(const git_error *) git_error_last(void);
-
-/**
- * Clear the last library error that occurred for this thread.
- */
-GIT_EXTERN(void) git_error_clear(void);
-
-/**
- * Set the error message string for this thread, using `printf`-style
- * formatting.
- *
- * This function is public so that custom ODB backends and the like can
- * relay an error message through libgit2.  Most regular users of libgit2
- * will never need to call this function -- actually, calling it in most
- * circumstances (for example, calling from within a callback function)
- * will just end up having the value overwritten by libgit2 internals.
+ * 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 error message is stored in thread-local storage and only applies
- * to the particular thread that this libgit2 call is made from.
+ * 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.)
  *
- * @param error_class One of the `git_error_t` enum above describing the
- *                    general subsystem that is responsible for the error.
- * @param fmt The `printf`-style format string; subsequent arguments must
- *            be the arguments for the format string.
- */
-GIT_EXTERN(void) git_error_set(int error_class, const char *fmt, ...)
-                 GIT_FORMAT_PRINTF(2, 3);
-
-/**
- * Set the error message string for this thread.  This function is like
- * `git_error_set` but takes a static string instead of a `printf`-style
- * format.
+ * The memory for this object is managed by libgit2. It should not
+ * be freed.
  *
- * @param error_class One of the `git_error_t` enum above describing the
- *                    general subsystem that is responsible for the error.
- * @param string The error message to keep
- * @return 0 on success or -1 on failure
- */
-GIT_EXTERN(int) git_error_set_str(int error_class, const char *string);
-
-/**
- * Set the error message to a special value for memory allocation failure.
- *
- * The normal `git_error_set_str()` function attempts to `strdup()` the
- * string that is passed in.  This is not a good idea when the error in
- * question is a memory allocation failure.  That circumstance has a
- * special setter function that sets the error string to a known and
- * statically allocated internal value.
+ * @return A git_error object.
  */
-GIT_EXTERN(void) git_error_set_oom(void);
+GIT_EXTERN(const git_error *) git_error_last(void);
 
 /** @} */
 GIT_END_DECL

include/git2/sys/errors.h

+/*
+ * Copyright (C) the libgit2 contributors. All rights reserved.
+ *
+ * This file is part of libgit2, distributed under the GNU GPL v2 with
+ * a Linking Exception. For full terms see the included COPYING file.
+ */
+
+#ifndef INCLUDE_sys_git_errors_h__
+#define INCLUDE_sys_git_errors_h__
+
+#include "git2/common.h"
+
+GIT_BEGIN_DECL
+
+/**
+ * Clear the last library error that occurred for this thread.
+ */
+GIT_EXTERN(void) git_error_clear(void);
+
+/**
+ * Set the error message string for this thread, using `printf`-style
+ * formatting.
+ *
+ * This function is public so that custom ODB backends and the like can
+ * relay an error message through libgit2.  Most regular users of libgit2
+ * will never need to call this function -- actually, calling it in most
+ * circumstances (for example, calling from within a callback function)
+ * will just end up having the value overwritten by libgit2 internals.
+ *
+ * This error message is stored in thread-local storage and only applies
+ * to the particular thread that this libgit2 call is made from.
+ *
+ * @param error_class One of the `git_error_t` enum above describing the
+ *                    general subsystem that is responsible for the error.
+ * @param fmt The `printf`-style format string; subsequent arguments must
+ *            be the arguments for the format string.
+ */
+GIT_EXTERN(void) git_error_set(int error_class, const char *fmt, ...)
+                 GIT_FORMAT_PRINTF(2, 3);
+
+/**
+ * Set the error message string for this thread.  This function is like
+ * `git_error_set` but takes a static string instead of a `printf`-style
+ * format.
+ *
+ * @param error_class One of the `git_error_t` enum above describing the
+ *                    general subsystem that is responsible for the error.
+ * @param string The error message to keep
+ * @return 0 on success or -1 on failure
+ */
+GIT_EXTERN(int) git_error_set_str(int error_class, const char *string);
+
+/**
+ * Set the error message to a special value for memory allocation failure.
+ *
+ * The normal `git_error_set_str()` function attempts to `strdup()` the
+ * string that is passed in.  This is not a good idea when the error in
+ * question is a memory allocation failure.  That circumstance has a
+ * special setter function that sets the error string to a known and
+ * statically allocated internal value.
+ */
+GIT_EXTERN(void) git_error_set_oom(void);
+
+GIT_END_DECL
+
+#endif

src/libgit2/clone.c

@@ -542,15 +542,15 @@ static int git__clone(
 	}
 
 	if (error != 0) {
-		git_error_state last_error = {0};
-		git_error_state_capture(&last_error, error);
+		git_error *last_error;
+		git_error_save(&last_error);
 
 		git_repository_free(repo);
 		repo = NULL;
 
 		(void)git_futils_rmdir_r(local_path, NULL, rmdir_flags);
 
-		git_error_state_restore(&last_error);
+		git_error_restore(last_error);
 	}
 
 	*out = repo;

src/libgit2/filter.c

@@ -908,7 +908,7 @@ static int buffered_stream_close(git_writestream *s)
 {
 	struct buffered_stream *buffered_stream = (struct buffered_stream *)s;
 	git_str *writebuf;
-	git_error_state error_state = {0};
+	git_error *last_error;
 	int error;
 
 	GIT_ASSERT_ARG(buffered_stream);
@@ -946,9 +946,9 @@ static int buffered_stream_close(git_writestream *s)
 	} else {
 		/* close stream before erroring out taking care
 		 * to preserve the original error */
-		git_error_state_capture(&error_state, error);
+		git_error_save(&last_error);
 		buffered_stream->target->close(buffered_stream->target);
-		git_error_state_restore(&error_state);
+		git_error_restore(last_error);
 		return error;
 	}
 

src/libgit2/index.c

@@ -1609,15 +1609,17 @@ int git_index_add_bypath(git_index *index, const char *path)
 
 	if (ret == GIT_EDIRECTORY) {
 		git_submodule *sm;
-		git_error_state err;
+		git_error *last_error;
 
-		git_error_state_capture(&err, ret);
+		git_error_save(&last_error);
 
 		ret = git_submodule_lookup(&sm, INDEX_OWNER(index), path);
-		if (ret == GIT_ENOTFOUND)
-			return git_error_state_restore(&err);
+		if (ret == GIT_ENOTFOUND) {
+			git_error_restore(last_error);
+			return GIT_EDIRECTORY;
+		}
 
-		git_error_state_free(&err);
+		git_error_free(last_error);
 
 		/*
 		 * EEXISTS means that there is a repository at that path, but it's not known

src/libgit2/libgit2.c

@@ -26,7 +26,6 @@
 #include "runtime.h"
 #include "sysdir.h"
 #include "thread.h"
-#include "threadstate.h"
 #include "git2/global.h"
 #include "streams/registry.h"
 #include "streams/mbedtls.h"
@@ -73,8 +72,9 @@ int git_libgit2_init(void)
 		git_win32_leakcheck_global_init,
 #endif
 		git_allocator_global_init,
-		git_threadstate_global_init,
+		git_error_global_init,
 		git_threads_global_init,
+		git_oid_global_init,
 		git_rand_global_init,
 		git_hash_global_init,
 		git_sysdir_global_init,

src/libgit2/oid.c

@@ -9,7 +9,7 @@
 
 #include "git2/oid.h"
 #include "repository.h"
-#include "threadstate.h"
+#include "runtime.h"
 #include <string.h>
 #include <limits.h>
 
@@ -153,15 +153,42 @@ int git_oid_pathfmt(char *str, const git_oid *oid)
 	return 0;
 }
 
+static git_tlsdata_key thread_str_key;
+
+static void GIT_SYSTEM_CALL thread_str_free(void *s)
+{
+	char *str = (char *)s;
+	git__free(str);
+}
+
+static void thread_str_global_shutdown(void)
+{
+	char *str = git_tlsdata_get(thread_str_key);
+	git_tlsdata_set(thread_str_key, NULL);
+
+	git__free(str);
+	git_tlsdata_dispose(thread_str_key);
+}
+
+int git_oid_global_init(void)
+{
+	if (git_tlsdata_init(&thread_str_key, thread_str_free) != 0)
+		return -1;
+
+	return git_runtime_shutdown_register(thread_str_global_shutdown);
+}
+
 char *git_oid_tostr_s(const git_oid *oid)
 {
-	git_threadstate *threadstate = git_threadstate_get();
 	char *str;
 
-	if (!threadstate)
-		return NULL;
+	if ((str = git_tlsdata_get(thread_str_key)) == NULL) {
+		if ((str = git__malloc(GIT_OID_MAX_HEXSIZE + 1)) == NULL)
+			return NULL;
+
+		git_tlsdata_set(thread_str_key, str);
+	}
 
-	str = threadstate->oid_fmt;
 	git_oid_nfmt(str, git_oid_hexsize(git_oid_type(oid)) + 1, oid);
 	return str;
 }

src/libgit2/oid.h

@@ -270,4 +270,6 @@ int git_oid__fromstrn(
 
 int git_oid__fromraw(git_oid *out, const unsigned char *raw, git_oid_t type);
 
+int git_oid_global_init(void);
+
 #endif

src/libgit2/threadstate.c

-/*
- * Copyright (C) the libgit2 contributors. All rights reserved.
- *
- * This file is part of libgit2, distributed under the GNU GPL v2 with
- * a Linking Exception. For full terms see the included COPYING file.
- */
-
-#include "threadstate.h"
-#include "runtime.h"
-
-/**
- * Handle the thread-local state
- *
- * `git_threadstate_global_init` will be called as part
- * of `git_libgit2_init` (which itself must be called
- * before calling any other function in the library).
- *
- * This function allocates a TLS index to store the per-
- * thread state.
- *
- * Any internal method that requires thread-local state
- * will then call `git_threadstate_get()` which returns a
- * pointer to the thread-local state structure; this
- * structure is lazily allocated on each thread.
- *
- * This mechanism will register a shutdown handler
- * (`git_threadstate_global_shutdown`) which will free the
- * TLS index.  This shutdown handler will be called by
- * `git_libgit2_shutdown`.
- */
-
-static git_tlsdata_key tls_key;
-
-static void threadstate_dispose(git_threadstate *threadstate)
-{
-	if (!threadstate)
-		return;
-
-	if (threadstate->error_t.message != git_str__initstr)
-		git__free(threadstate->error_t.message);
-	threadstate->error_t.message = NULL;
-}
-
-static void GIT_SYSTEM_CALL threadstate_free(void *threadstate)
-{
-	threadstate_dispose(threadstate);
-	git__free(threadstate);
-}
-
-static void git_threadstate_global_shutdown(void)
-{
-	git_threadstate *threadstate;
-
-	threadstate = git_tlsdata_get(tls_key);
-	git_tlsdata_set(tls_key, NULL);
-
-	threadstate_dispose(threadstate);
-	git__free(threadstate);
-
-	git_tlsdata_dispose(tls_key);
-}
-
-int git_threadstate_global_init(void)
-{
-	if (git_tlsdata_init(&tls_key, &threadstate_free) != 0)
-		return -1;
-
-	return git_runtime_shutdown_register(git_threadstate_global_shutdown);
-}
-
-git_threadstate *git_threadstate_get(void)
-{
-	git_threadstate *threadstate;
-
-	if ((threadstate = git_tlsdata_get(tls_key)) != NULL)
-		return threadstate;
-
-	/*
-	 * Avoid git__malloc here, since if it fails, it sets an error
-	 * message, which requires thread state, which would allocate
-	 * here, which would fail, which would set an error message...
-	 */
-
-	if ((threadstate = git__allocator.gmalloc(sizeof(git_threadstate),
-			__FILE__, __LINE__)) == NULL)
-		return NULL;
-
-	memset(threadstate, 0, sizeof(git_threadstate));
-
-	if (git_str_init(&threadstate->error_buf, 0) < 0) {
-		git__allocator.gfree(threadstate);
-		return NULL;
-	}
-
-	git_tlsdata_set(tls_key, threadstate);
-	return threadstate;
-}

src/libgit2/threadstate.h

-/*
- * Copyright (C) the libgit2 contributors. All rights reserved.
- *
- * This file is part of libgit2, distributed under the GNU GPL v2 with
- * a Linking Exception. For full terms see the included COPYING file.
- */
-#ifndef INCLUDE_threadstate_h__
-#define INCLUDE_threadstate_h__
-
-#include "common.h"
-
-typedef struct {
-	git_error *last_error;
-	git_error error_t;
-	git_str error_buf;
-	char oid_fmt[GIT_OID_MAX_HEXSIZE+1];
-} git_threadstate;
-
-extern int git_threadstate_global_init(void);
-extern git_threadstate *git_threadstate_get(void);
-
-#endif

src/libgit2/transports/httpclient.c

@@ -768,25 +768,37 @@ static int check_certificate(
 	void *cert_cb_payload)
 {
 	git_cert *cert;
-	git_error_state last_error = {0};
+	git_error *last_error;
 	int error;
 
 	if ((error = git_stream_certificate(&cert, stream)) < 0)
 		return error;
 
-	git_error_state_capture(&last_error, GIT_ECERTIFICATE);
+	/*
+	 * Allow callers to set an error - but save ours and clear
+	 * it, so that we can detect if they set one and restore it
+	 * if we need to.
+	 */
+	git_error_save(&last_error);
+	git_error_clear();
 
 	error = cert_cb(cert, is_valid, url->host, cert_cb_payload);
 
-	if (error == GIT_PASSTHROUGH && !is_valid)
-		return git_error_state_restore(&last_error);
-	else if (error == GIT_PASSTHROUGH)
-		error = 0;
-	else if (error && !git_error_last())
-		git_error_set(GIT_ERROR_HTTP,
-		              "user rejected certificate for %s", url->host);
+	if (error == GIT_PASSTHROUGH) {
+		error = is_valid ? 0 : -1;
+
+		if (error) {
+			git_error_restore(last_error);
+			last_error = NULL;
+		}
+	} else if (error) {
+		if (!git_error_exists())
+			git_error_set(GIT_ERROR_HTTP,
+		              "user rejected certificate for %s",
+			      url->host);
+	}
 
-	git_error_state_free(&last_error);
+	git_error_free(last_error);
 	return error;
 }
 

src/libgit2/transports/ssh_libssh2.c

@@ -665,7 +665,7 @@ static int check_certificate(
 	git_cert_hostkey cert = {{ 0 }};
 	const char *key;
 	size_t cert_len;
-	int cert_type, cert_valid = 0, error = 0;
+	int cert_type, cert_valid = 0, error = GIT_ECERTIFICATE;
 
 	if ((key = libssh2_session_hostkey(session, &cert_len, &cert_type)) == NULL) {
 		ssh_error(session, "failed to retrieve hostkey");
@@ -735,29 +735,24 @@ static int check_certificate(
 		return -1;
 	}
 
-	git_error_clear();
-	error = 0;
-	if (!cert_valid) {
-		git_error_set(GIT_ERROR_SSH, "invalid or unknown remote ssh hostkey");
-		error = GIT_ECERTIFICATE;
-	}
-
 	if (check_cb != NULL) {
 		git_cert_hostkey *cert_ptr = &cert;
-		git_error_state previous_error = {0};
 
-		git_error_state_capture(&previous_error, error);
-		error = check_cb((git_cert *) cert_ptr, cert_valid, host, check_cb_payload);
-		if (error == GIT_PASSTHROUGH) {
-			error = git_error_state_restore(&previous_error);
-		} else if (error < 0 && !git_error_last()) {
-			git_error_set(GIT_ERROR_NET, "unknown remote host key");
-		}
+		error = check_cb((git_cert *)cert_ptr, cert_valid, host,
+			         check_cb_payload);
 
-		git_error_state_free(&previous_error);
+		if (error == 0)
+			cert_valid = 1;
+		else if (error != GIT_PASSTHROUGH)
+			cert_valid = 0;
 	}
 
-	return error;
+	if (!cert_valid) {
+		git_error_set(GIT_ERROR_SSH, "invalid or unknown remote ssh hostkey");
+		return (error == GIT_PASSTHROUGH) ? GIT_ECERTIFICATE : error;
+	}
+
+	return 0;
 }
 
 #define SSH_DEFAULT_PORT "22"

src/libgit2/errors.c → src/util/errors.c

@@ -5,16 +5,17 @@
  * a Linking Exception. For full terms see the included COPYING file.
  */
 
-#include "common.h"
+#include "git2_util.h"
 
-#include "threadstate.h"
+#include "errors.h"
 #include "posix.h"
 #include "str.h"
-#include "libgit2.h"
+#include "runtime.h"
 
-/********************************************
- * New error handling
- ********************************************/
+/*
+ * Some static error data that is used when we're out of memory, TLS
+ * has not been setup, or TLS has failed.
+ */
 
 static git_error oom_error = {
 	"Out of memory",
@@ -22,61 +23,151 @@ static git_error oom_error = {
 };
 
 static git_error uninitialized_error = {
-	"libgit2 has not been initialized; you must call git_libgit2_init",
+	"library has not been initialized",
 	GIT_ERROR_INVALID
 };
 
 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
+};
+
+#define IS_STATIC_ERROR(err) \
+	((err) == &oom_error || (err) == &uninitialized_error || \
+	 (err) == &tlsdata_error || (err) == &no_error)
+
+/* Per-thread error state (TLS) */
+
+static git_tlsdata_key tls_key;
+
+struct error_threadstate {
+	/* The error message buffer. */
+	git_str message;
+
+	/* Error information, set by `git_error_set` and friends. */
+	git_error error;
+
+	/*
+	 * The last error to occur; points to the error member of this
+	 * struct _or_ a static error.
+	 */
+	git_error *last;
 };
 
+static void threadstate_dispose(struct error_threadstate *threadstate)
+{
+	if (!threadstate)
+		return;
+
+	git_str_dispose(&threadstate->message);
+}
+
+static struct error_threadstate *threadstate_get(void)
+{
+	struct error_threadstate *threadstate;
+
+	if ((threadstate = git_tlsdata_get(tls_key)) != NULL)
+		return threadstate;
+
+	/*
+	 * Avoid git__malloc here, since if it fails, it sets an error
+	 * message, which requires thread state, which would allocate
+	 * here, which would fail, which would set an error message...
+	 */
+
+	if ((threadstate = git__allocator.gmalloc(
+			sizeof(struct error_threadstate),
+			__FILE__, __LINE__)) == NULL)
+		return NULL;
+
+	memset(threadstate, 0, sizeof(struct error_threadstate));
+
+	if (git_str_init(&threadstate->message, 0) < 0) {
+		git__allocator.gfree(threadstate);
+		return NULL;
+	}
+
+	git_tlsdata_set(tls_key, threadstate);
+	return threadstate;
+}
+
+static void GIT_SYSTEM_CALL threadstate_free(void *threadstate)
+{
+	threadstate_dispose(threadstate);
+	git__free(threadstate);
+}
+
+static void git_error_global_shutdown(void)
+{
+	struct error_threadstate *threadstate;
+
+	threadstate = git_tlsdata_get(tls_key);
+	git_tlsdata_set(tls_key, NULL);
+
+	threadstate_dispose(threadstate);
+	git__free(threadstate);
+
+	git_tlsdata_dispose(tls_key);
+}
+
+int git_error_global_init(void)
+{
+	if (git_tlsdata_init(&tls_key, &threadstate_free) != 0)
+		return -1;
+
+	return git_runtime_shutdown_register(git_error_global_shutdown);
+}
+
 static void set_error_from_buffer(int error_class)
 {
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 	git_error *error;
 	git_str *buf;
 
 	if (!threadstate)
 		return;
 
-	error = &threadstate->error_t;
-	buf = &threadstate->error_buf;
+	error = &threadstate->error;
+	buf = &threadstate->message;
 
 	error->message = buf->ptr;
 	error->klass = error_class;
 
-	threadstate->last_error = error;
+	threadstate->last = error;
 }
 
 static void set_error(int error_class, char *string)
 {
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 	git_str *buf;
 
 	if (!threadstate)
 		return;
 
-	buf = &threadstate->error_buf;
+	buf = &threadstate->message;
 
 	git_str_clear(buf);
 
-	if (string) {
+	if (string)
 		git_str_puts(buf, string);
-		git__free(string);
-	}
 
-	set_error_from_buffer(error_class);
+	if (!git_str_oom(buf))
+		set_error_from_buffer(error_class);
 }
 
 void git_error_set_oom(void)
 {
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 
 	if (!threadstate)
 		return;
 
-	threadstate->last_error = &oom_error;
+	threadstate->last = &oom_error;
 }
 
 void git_error_set(int error_class, const char *fmt, ...)
@@ -94,14 +185,14 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 	DWORD win32_error_code = (error_class == GIT_ERROR_OS) ? GetLastError() : 0;
 #endif
 
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 	int error_code = (error_class == GIT_ERROR_OS) ? errno : 0;
 	git_str *buf;
 
 	if (!threadstate)
 		return;
 
-	buf = &threadstate->error_buf;
+	buf = &threadstate->message;
 
 	git_str_clear(buf);
 
@@ -135,7 +226,7 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 
 int git_error_set_str(int error_class, const char *string)
 {
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 	git_str *buf;
 
 	GIT_ASSERT_ARG(string);
@@ -143,7 +234,7 @@ int git_error_set_str(int error_class, const char *string)
 	if (!threadstate)
 		return -1;
 
-	buf = &threadstate->error_buf;
+	buf = &threadstate->message;
 
 	git_str_clear(buf);
 	git_str_puts(buf, string);
@@ -157,14 +248,14 @@ int git_error_set_str(int error_class, const char *string)
 
 void git_error_clear(void)
 {
-	git_threadstate *threadstate = git_threadstate_get();
+	struct error_threadstate *threadstate = threadstate_get();
 
 	if (!threadstate)
 		return;
 
-	if (threadstate->last_error != NULL) {
+	if (threadstate->last != NULL) {
 		set_error(0, NULL);
-		threadstate->last_error = NULL;
+		threadstate->last = NULL;
 	}
 
 	errno = 0;
@@ -173,81 +264,95 @@ void git_error_clear(void)
 #endif
 }
 
+bool git_error_exists(void)
+{
+	struct error_threadstate *threadstate;
+
+	if ((threadstate = threadstate_get()) == NULL)
+		return true;
+
+	return threadstate->last != NULL;
+}
+
 const git_error *git_error_last(void)
 {
-	git_threadstate *threadstate;
+	struct error_threadstate *threadstate;
 
 	/* If the library is not initialized, return a static error. */
-	if (!git_libgit2_init_count())
+	if (!git_runtime_init_count())
 		return &uninitialized_error;
 
-	if ((threadstate = git_threadstate_get()) == NULL)
+	if ((threadstate = threadstate_get()) == NULL)
 		return &tlsdata_error;
 
-	return threadstate->last_error;
+	if (!threadstate->last)
+		return &no_error;
+
+	return threadstate->last;
 }
 
-int git_error_state_capture(git_error_state *state, int error_code)
+int git_error_save(git_error **out)
 {
-	git_threadstate *threadstate = git_threadstate_get();
-	git_error *error;
-	git_str *error_buf;
+	struct error_threadstate *threadstate = threadstate_get();
+	git_error *error, *dup;
 
-	if (!threadstate)
+	if (!threadstate) {
+		*out = &tlsdata_error;
 		return -1;
+	}
 
-	error = threadstate->last_error;
-	error_buf = &threadstate->error_buf;
-
-	memset(state, 0, sizeof(git_error_state));
+	error = threadstate->last;
 
-	if (!error_code)
+	if (!error || error == &no_error) {
+		*out = &no_error;
 		return 0;
+	} else if (IS_STATIC_ERROR(error)) {
+		*out = error;
+		return 0;
+	}
 
-	state->error_code = error_code;
-	state->oom = (error == &oom_error);
+	if ((dup = git__malloc(sizeof(git_error))) == NULL) {
+		*out = &oom_error;
+		return -1;
+	}
 
-	if (error) {
-		state->error_msg.klass = error->klass;
+	dup->klass = error->klass;
+	dup->message = git__strdup(error->message);
 
-		if (state->oom)
-			state->error_msg.message = oom_error.message;
-		else
-			state->error_msg.message = git_str_detach(error_buf);
+	if (!dup->message) {
+		*out = &oom_error;
+		return -1;
 	}
 
-	git_error_clear();
-	return error_code;
+	*out = dup;
+	return 0;
 }
 
-int git_error_state_restore(git_error_state *state)
+int git_error_restore(git_error *error)
 {
-	int ret = 0;
-
-	git_error_clear();
+	struct error_threadstate *threadstate = threadstate_get();
 
-	if (state && state->error_msg.message) {
-		if (state->oom)
-			git_error_set_oom();
-		else
-			set_error(state->error_msg.klass, state->error_msg.message);
+	GIT_ASSERT_ARG(error);
 
-		ret = state->error_code;
-		memset(state, 0, sizeof(git_error_state));
-	}
+	if (IS_STATIC_ERROR(error) && threadstate)
+		threadstate->last = error;
+	else
+		set_error(error->klass, error->message);
 
-	return ret;
+	git_error_free(error);
+	return 0;
 }
 
-void git_error_state_free(git_error_state *state)
+void git_error_free(git_error *error)
 {
-	if (!state)
+	if (!error)
 		return;
 
-	if (!state->oom)
-		git__free(state->error_msg.message);
+	if (IS_STATIC_ERROR(error))
+		return;
 
-	memset(state, 0, sizeof(git_error_state));
+	git__free(error->message);
+	git__free(error);
 }
 
 int git_error_system_last(void)

src/libgit2/errors.h → src/util/errors.h

@@ -8,7 +8,11 @@
 #ifndef INCLUDE_errors_h__
 #define INCLUDE_errors_h__
 
-#include "common.h"
+#include "git2_util.h"
+#include "git2/sys/errors.h"
+
+/* Initialize the error thread-state. */
+int git_error_global_init(void);
 
 /*
  * `vprintf`-style formatting for the error message for this thread.
@@ -16,6 +20,11 @@
 void git_error_vset(int error_class, const char *fmt, va_list ap);
 
 /**
+ * Determines whether an error exists.
+ */
+bool git_error_exists(void);
+
+/**
  * Set error message for user callback if needed.
  *
  * If the error code in non-zero and no error message is set, this
@@ -27,9 +36,8 @@ GIT_INLINE(int) git_error_set_after_callback_function(
 	int error_code, const char *action)
 {
 	if (error_code) {
-		const git_error *e = git_error_last();
-		if (!e || !e->message)
-			git_error_set(e ? e->klass : GIT_ERROR_CALLBACK,
+		if (!git_error_exists())
+			git_error_set(GIT_ERROR_CALLBACK,
 				"%s callback returned %d", action, error_code);
 	}
 	return error_code;
@@ -54,27 +62,23 @@ int git_error_system_last(void);
 void git_error_system_set(int code);
 
 /**
- * Structure to preserve libgit2 error state
- */
-typedef struct {
-	int error_code;
-	unsigned int oom : 1;
-	git_error error_msg;
-} git_error_state;
-
-/**
  * Capture current error state to restore later, returning error code.
  * If `error_code` is zero, this does not clear the current error state.
  * You must either restore this error state, or free it.
+ *
+ * This function returns 0 on success, or -1 on failure. If the function
+ * fails, the `out` structure is set to the failure error message and
+ * the normal system error message is not updated.
  */
-extern int git_error_state_capture(git_error_state *state, int error_code);
+extern int git_error_save(git_error **out);
 
 /**
- * Restore error state to a previous value, returning saved error code.
+ * Restore thread error state to the given value. The given value is
+ * freed and `git_error_free` need not be called on it.
  */
-extern int git_error_state_restore(git_error_state *state);
+extern int git_error_restore(git_error *error);
 
 /** Free an error state. */
-extern void git_error_state_free(git_error_state *state);
+extern void git_error_free(git_error *error);
 
 #endif

src/util/git2_util.h

@@ -12,6 +12,7 @@
 #endif
 
 #include "git2/common.h"
+#include "git2/sys/errors.h"
 #include "cc-compat.h"
 
 typedef struct git_str git_str;

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;
@@ -88,52 +97,32 @@ void test_errors__new_school(void)
 
 void test_errors__restore(void)
 {
-	git_error_state err_state = {0};
+	git_error *last_error;
 
 	git_error_clear();
-	cl_assert(git_error_last() == NULL);
-
-	cl_assert_equal_i(0, git_error_state_capture(&err_state, 0));
-
-	memset(&err_state, 0x0, sizeof(git_error_state));
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp("no error", git_error_last()->message) == 0);
 
 	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);
-
-	git_error_set(99, "Bar: %s", "foo");
-
-	git_error_state_restore(&err_state);
-
-	cl_assert_equal_i(42, git_error_last()->klass);
-	cl_assert_equal_s("Foo: bar", git_error_last()->message);
-}
-
-void test_errors__free_state(void)
-{
-	git_error_state err_state = {0};
+	cl_assert(git_error_save(&last_error) == 0);
 
 	git_error_clear();
-
-	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()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp("no error", git_error_last()->message) == 0);
 
 	git_error_set(99, "Bar: %s", "foo");
 
-	git_error_state_free(&err_state);
-
-	cl_assert_equal_i(99, git_error_last()->klass);
-	cl_assert_equal_s("Bar: foo", git_error_last()->message);
+	git_error_restore(last_error);
 
-	git_error_state_restore(&err_state);
-
-	cl_assert(git_error_last() == NULL);
+	cl_assert(git_error_last()->klass == 42);
+	cl_assert(strcmp("Foo: bar", git_error_last()->message) == 0);
 }
 
 void test_errors__restore_oom(void)
 {
-	git_error_state err_state = {0};
+	git_error *last_error;
 	const git_error *oom_error = NULL;
 
 	git_error_clear();
@@ -141,15 +130,18 @@ void test_errors__restore_oom(void)
 	git_error_set_oom(); /* internal fn */
 	oom_error = git_error_last();
 	cl_assert(oom_error);
+	cl_assert(oom_error->klass == GIT_ERROR_NOMEMORY);
 
-	cl_assert_equal_i(-1, git_error_state_capture(&err_state, -1));
-
-	cl_assert(git_error_last() == NULL);
-	cl_assert_equal_i(GIT_ERROR_NOMEMORY, err_state.error_msg.klass);
-	cl_assert_equal_s("Out of memory", err_state.error_msg.message);
+	cl_assert(git_error_save(&last_error) == 0);
+	cl_assert(last_error->klass == GIT_ERROR_NOMEMORY);
+	cl_assert(strcmp("Out of memory", last_error->message) == 0);
 
-	git_error_state_restore(&err_state);
+	git_error_clear();
+	cl_assert(git_error_last() != NULL);
+	cl_assert(git_error_last()->klass == GIT_ERROR_NONE);
+	cl_assert(strcmp("no error", git_error_last()->message) == 0);
 
+	git_error_restore(last_error);
 	cl_assert(git_error_last()->klass == GIT_ERROR_NOMEMORY);
 	cl_assert_(git_error_last() == oom_error, "static oom error not restored");
 
@@ -204,11 +196,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));