Merge pull request libgit2#5722 from libgit2/ethomson/tlsdata_fail

Thread-local storage: handle failure cases

Author: ethomson

include/git2/oid.h

@@ -225,7 +225,7 @@ GIT_EXTERN(int) git_oid_pathfmt(char *out, const git_oid *id);
  * concurrent calls of the function.
  *
  * @param oid The oid structure to format
- * @return the c-string
+ * @return the c-string or NULL on failure
  */
 GIT_EXTERN(char *) git_oid_tostr_s(const git_oid *oid);
 

src/libgit2/errors.c

@@ -16,32 +16,51 @@
  * New error handling
  ********************************************/
 
-static git_error g_git_oom_error = {
+static git_error oom_error = {
 	"Out of memory",
 	GIT_ERROR_NOMEMORY
 };
 
-static git_error g_git_uninitialized_error = {
+static git_error uninitialized_error = {
 	"libgit2 has not been initialized; you must call git_libgit2_init",
 	GIT_ERROR_INVALID
 };
 
+static git_error tlsdata_error = {
+	"thread-local data initialization failure",
+	GIT_ERROR
+};
+
 static void set_error_from_buffer(int error_class)
 {
-	git_error *error = &GIT_THREADSTATE->error_t;
-	git_str *buf = &GIT_THREADSTATE->error_buf;
+	git_threadstate *threadstate = git_threadstate_get();
+	git_error *error;
+	git_str *buf;
+
+	if (!threadstate)
+		return;
+
+	error = &threadstate->error_t;
+	buf = &threadstate->error_buf;
 
 	error->message = buf->ptr;
 	error->klass = error_class;
 
-	GIT_THREADSTATE->last_error = error;
+	threadstate->last_error = error;
 }
 
 static void set_error(int error_class, char *string)
 {
-	git_str *buf = &GIT_THREADSTATE->error_buf;
+	git_threadstate *threadstate = git_threadstate_get();
+	git_str *buf;
+
+	if (!threadstate)
+		return;
+
+	buf = &threadstate->error_buf;
 
 	git_str_clear(buf);
+
 	if (string) {
 		git_str_puts(buf, string);
 		git__free(string);
@@ -52,7 +71,12 @@ static void set_error(int error_class, char *string)
 
 void git_error_set_oom(void)
 {
-	GIT_THREADSTATE->last_error = &g_git_oom_error;
+	git_threadstate *threadstate = git_threadstate_get();
+
+	if (!threadstate)
+		return;
+
+	threadstate->last_error = &oom_error;
 }
 
 void git_error_set(int error_class, const char *fmt, ...)
@@ -69,10 +93,18 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 #ifdef GIT_WIN32
 	DWORD win32_error_code = (error_class == GIT_ERROR_OS) ? GetLastError() : 0;
 #endif
+
+	git_threadstate *threadstate = git_threadstate_get();
 	int error_code = (error_class == GIT_ERROR_OS) ? errno : 0;
-	git_str *buf = &GIT_THREADSTATE->error_buf;
+	git_str *buf;
+
+	if (!threadstate)
+		return;
+
+	buf = &threadstate->error_buf;
 
 	git_str_clear(buf);
+
 	if (fmt) {
 		git_str_vprintf(buf, fmt, ap);
 		if (error_class == GIT_ERROR_OS)
@@ -81,7 +113,7 @@ void git_error_vset(int error_class, const char *fmt, va_list ap)
 
 	if (error_class == GIT_ERROR_OS) {
 #ifdef GIT_WIN32
-		char * win32_error = git_win32_get_error_message(win32_error_code);
+		char *win32_error = git_win32_get_error_message(win32_error_code);
 		if (win32_error) {
 			git_str_puts(buf, win32_error);
 			git__free(win32_error);
@@ -103,10 +135,16 @@ 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_str *buf = &GIT_THREADSTATE->error_buf;
+	git_threadstate *threadstate = git_threadstate_get();
+	git_str *buf;
 
 	GIT_ASSERT_ARG(string);
 
+	if (!threadstate)
+		return -1;
+
+	buf = &threadstate->error_buf;
+
 	git_str_clear(buf);
 	git_str_puts(buf, string);
 
@@ -119,9 +157,14 @@ int git_error_set_str(int error_class, const char *string)
 
 void git_error_clear(void)
 {
-	if (GIT_THREADSTATE->last_error != NULL) {
+	git_threadstate *threadstate = git_threadstate_get();
+
+	if (!threadstate)
+		return;
+
+	if (threadstate->last_error != NULL) {
 		set_error(0, NULL);
-		GIT_THREADSTATE->last_error = NULL;
+		threadstate->last_error = NULL;
 	}
 
 	errno = 0;
@@ -132,31 +175,43 @@ void git_error_clear(void)
 
 const git_error *git_error_last(void)
 {
+	git_threadstate *threadstate;
+
 	/* If the library is not initialized, return a static error. */
 	if (!git_libgit2_init_count())
-		return &g_git_uninitialized_error;
+		return &uninitialized_error;
+
+	if ((threadstate = git_threadstate_get()) == NULL)
+		return &tlsdata_error;
 
-	return GIT_THREADSTATE->last_error;
+	return threadstate->last_error;
 }
 
 int git_error_state_capture(git_error_state *state, int error_code)
 {
-	git_error *error = GIT_THREADSTATE->last_error;
-	git_str *error_buf = &GIT_THREADSTATE->error_buf;
+	git_threadstate *threadstate = git_threadstate_get();
+	git_error *error;
+	git_str *error_buf;
+
+	if (!threadstate)
+		return -1;
+
+	error = threadstate->last_error;
+	error_buf = &threadstate->error_buf;
 
 	memset(state, 0, sizeof(git_error_state));
 
 	if (!error_code)
 		return 0;
 
 	state->error_code = error_code;
-	state->oom = (error == &g_git_oom_error);
+	state->oom = (error == &oom_error);
 
 	if (error) {
 		state->error_msg.klass = error->klass;
 
 		if (state->oom)
-			state->error_msg.message = g_git_oom_error.message;
+			state->error_msg.message = oom_error.message;
 		else
 			state->error_msg.message = git_str_detach(error_buf);
 	}

src/libgit2/indexer.c

@@ -519,7 +519,13 @@ static int store_object(git_indexer *idx)
 	pentry->offset = entry_start;
 
 	if (git_oidmap_exists(idx->pack->idx_cache, &pentry->id)) {
-		git_error_set(GIT_ERROR_INDEXER, "duplicate object %s found in pack", git_oid_tostr_s(&pentry->id));
+		const char *idstr = git_oid_tostr_s(&pentry->id);
+
+		if (!idstr)
+			git_error_set(GIT_ERROR_INDEXER, "failed to parse object id");
+		else
+			git_error_set(GIT_ERROR_INDEXER, "duplicate object %s found in pack", idstr);
+
 		git__free(pentry);
 		goto on_error;
 	}

src/libgit2/odb.c

@@ -1494,11 +1494,16 @@ static int read_prefix_1(git_odb_object **out, git_odb *db,
 
 			if (found && git_oid__cmp(&full_oid, &found_full_oid)) {
 				git_str buf = GIT_STR_INIT;
+				const char *idstr;
 
-				git_str_printf(&buf, "multiple matches for prefix: %s",
-					git_oid_tostr_s(&full_oid));
-				git_str_printf(&buf, " %s",
-					git_oid_tostr_s(&found_full_oid));
+				if ((idstr = git_oid_tostr_s(&full_oid)) == NULL) {
+					git_str_puts(&buf, "failed to parse object id");
+				} else {
+					git_str_printf(&buf, "multiple matches for prefix: %s", idstr);
+
+					if ((idstr = git_oid_tostr_s(&found_full_oid)) != NULL)
+						git_str_printf(&buf, " %s", idstr);
+				}
 
 				error = git_odb__error_ambiguous(buf.ptr);
 				git_str_dispose(&buf);

src/libgit2/oid.c

@@ -155,7 +155,13 @@ int git_oid_pathfmt(char *str, const git_oid *oid)
 
 char *git_oid_tostr_s(const git_oid *oid)
 {
-	char *str = GIT_THREADSTATE->oid_fmt;
+	git_threadstate *threadstate = git_threadstate_get();
+	char *str;
+
+	if (!threadstate)
+		return NULL;
+
+	str = threadstate->oid_fmt;
 	git_oid_nfmt(str, git_oid_hexsize(git_oid_type(oid)) + 1, oid);
 	return str;
 }

src/libgit2/repository.c

@@ -3422,12 +3422,18 @@ int git_repository_hashfile(
 
 static int checkout_message(git_str *out, git_reference *old, const char *new)
 {
+	const char *idstr;
+
 	git_str_puts(out, "checkout: moving from ");
 
-	if (git_reference_type(old) == GIT_REFERENCE_SYMBOLIC)
+	if (git_reference_type(old) == GIT_REFERENCE_SYMBOLIC) {
 		git_str_puts(out, git_reference__shorthand(git_reference_symbolic_target(old)));
-	else
-		git_str_puts(out, git_oid_tostr_s(git_reference_target(old)));
+	} else {
+		if ((idstr = git_oid_tostr_s(git_reference_target(old))) == NULL)
+			return -1;
+
+		git_str_puts(out, idstr);
+	}
 
 	git_str_puts(out, " to ");
 
@@ -3463,8 +3469,11 @@ static int detach(git_repository *repo, const git_oid *id, const char *new)
 	if ((error = git_object_peel(&peeled, object, GIT_OBJECT_COMMIT)) < 0)
 		goto cleanup;
 
-	if (new == NULL)
-		new = git_oid_tostr_s(git_object_id(peeled));
+	if (new == NULL &&
+	    (new = git_oid_tostr_s(git_object_id(peeled))) == NULL) {
+		error = -1;
+		goto cleanup;
+	}
 
 	if ((error = checkout_message(&log_message, current, new)) < 0)
 		goto cleanup;
@@ -3552,6 +3561,7 @@ int git_repository_detach_head(git_repository *repo)
 	git_reference *old_head = NULL,	*new_head = NULL, *current = NULL;
 	git_object *object = NULL;
 	git_str log_message = GIT_STR_INIT;
+	const char *idstr;
 	int error;
 
 	GIT_ASSERT_ARG(repo);
@@ -3565,7 +3575,12 @@ int git_repository_detach_head(git_repository *repo)
 	if ((error = git_object_lookup(&object, repo, git_reference_target(old_head), GIT_OBJECT_COMMIT)) < 0)
 		goto cleanup;
 
-	if ((error = checkout_message(&log_message, current, git_oid_tostr_s(git_object_id(object)))) < 0)
+	if ((idstr = git_oid_tostr_s(git_object_id(object))) == NULL) {
+		error = -1;
+		goto cleanup;
+	}
+
+	if ((error = checkout_message(&log_message, current, idstr)) < 0)
 		goto cleanup;
 
 	error = git_reference_create(&new_head, repo, GIT_HEAD_FILE, git_reference_target(old_head),

src/libgit2/threadstate.h

@@ -19,6 +19,4 @@ typedef struct {
 extern int git_threadstate_global_init(void);
 extern git_threadstate *git_threadstate_get(void);
 
-#define GIT_THREADSTATE (git_threadstate_get())
-
 #endif