During recent discussion of histedit changes an issue with error handling
came up. If a function handles two errors at once, it is possible that
both errors end up being stored in the same 'static struct got_error',
in which case one error will be overwritten.

In the case under discussion, the user saw "object not found" while the
expected error text was something like "log message cannot be empty".
This could happen in all sorts of ways. The current error message system
will yield unpredictable results depending on how it is used.

The patch below adds an array of pre-allocated errors which we can cycle
through in order to prevent this problem, at least in single-threaded
contexts. The problem will now only happen if more errors are in flight
than fit into the array. And that's not going to be a problem in practice.

I realize that this is not a thread-safe solution due to lack of any atomic
operations and locking. We do need a thread-safe solution for tog(1).

I don't want to wire pthreads into this part of the program. And I'd really
like to avoid any dynamic memory allocation because I want this code to be
able to operate under ENOMEM conditions. And not having to worry about
errors leaking memory is nice.

Declaring the array index as an atomic_int from stdatomic.h would be an
ideal solution from my point of view. I've tested this approach on OpenBSD
and it works. The caveat is that this is a gcc/clang extension, and an
optional feature of C11. For now, the index is a plain unsigned int.
Are there any concerns about using atomic_int?

Another thread-safety problem is that the code users strerror() but
switching to strerror_r() on top of the changes below would be trivial.

diff 024c8833f087804d46239c8919147870ab57242b 9969f4a0944bd8fc2c9945d8d94e9f3d7d4cb13f
blob - 772c50ed2ea44fc86f4cb411b60f6bbf55ef0a88
blob + a41a721678814c6e3adef116e9e6445a894d022a
--- lib/error.c
+++ lib/error.c
@@ -38,6 +38,18 @@
 #define nitems(_a) (sizeof(_a) / sizeof((_a)[0]))
 #endif
 
+static struct got_custom_error {
+	struct got_error err;
+	char msg[4080];
+} custom_errors[16];
+
+static struct got_custom_error *
+get_custom_err(void)
+{
+	static unsigned int idx;
+	return &custom_errors[(idx++) % nitems(custom_errors)];
+}
+
 const struct got_error *
 got_error(int code)
 {
@@ -54,14 +66,16 @@ got_error(int code)
 const struct got_error *
 got_error_msg(int code, const char *msg)
 {
-	static struct got_error err;
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 	size_t i;
 
 	for (i = 0; i < nitems(got_errors); i++) {
 		if (code == got_errors[i].code) {
-			err.code = code;
-			err.msg = msg;
-			return &err;
+			err->code = code;
+			strlcpy(cerr->msg, msg, sizeof(cerr->msg));
+			err->msg = cerr->msg;
+			return err;
 		}
 	}
 
@@ -71,51 +85,51 @@ got_error_msg(int code, const char *msg)
 const struct got_error *
 got_error_from_errno(const char *prefix)
 {
-	static struct got_error err;
-	static char err_msg[PATH_MAX + 20];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 
-	snprintf(err_msg, sizeof(err_msg), "%s: %s", prefix,
+	snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s", prefix,
 	    strerror(errno));
 
-	err.code = GOT_ERR_ERRNO;
-	err.msg = err_msg;
-	return &err;
+	err->code = GOT_ERR_ERRNO;
+	err->msg = cerr->msg;
+	return err;
 }
 
 const struct got_error *
 got_error_from_errno2(const char *prefix, const char *prefix2)
 {
-	static struct got_error err;
-	static char err_msg[(PATH_MAX * 2) + 20];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 
-	snprintf(err_msg, sizeof(err_msg), "%s: %s: %s", prefix, prefix2,
+	snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s: %s", prefix, prefix2,
 	    strerror(errno));
 
-	err.code = GOT_ERR_ERRNO;
-	err.msg = err_msg;
-	return &err;
+	err->code = GOT_ERR_ERRNO;
+	err->msg = cerr->msg;
+	return err;
 }
 
 const struct got_error *
 got_error_from_errno3(const char *prefix, const char *prefix2,
     const char *prefix3)
 {
-	static struct got_error err;
-	static char err_msg[(PATH_MAX * 3) + 20];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 
-	snprintf(err_msg, sizeof(err_msg), "%s: %s: %s: %s", prefix, prefix2,
-	    prefix3, strerror(errno));
+	snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s: %s: %s", prefix,
+	    prefix2, prefix3, strerror(errno));
 
-	err.code = GOT_ERR_ERRNO;
-	err.msg = err_msg;
-	return &err;
+	err->code = GOT_ERR_ERRNO;
+	err->msg = cerr->msg;
+	return err;
 }
 
 const struct got_error *
 got_error_from_errno_fmt(const char *fmt, ...)
 {
-	static struct got_error err;
-	static char err_msg[PATH_MAX * 4 + 64];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 	char buf[PATH_MAX * 4];
 	va_list ap;
 
@@ -123,11 +137,11 @@ got_error_from_errno_fmt(const char *fmt, ...)
 	vsnprintf(buf, sizeof(buf), fmt, ap);
 	va_end(ap);
 
-	snprintf(err_msg, sizeof(err_msg), "%s: %s", buf, strerror(errno));
+	snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s", buf, strerror(errno));
 
-	err.code = GOT_ERR_ERRNO;
-	err.msg = err_msg;
-	return &err;
+	err->code = GOT_ERR_ERRNO;
+	err->msg = cerr->msg;
+	return err;
 }
 
 const struct got_error *
@@ -148,8 +162,7 @@ got_ferror(FILE *f, int code)
 const struct got_error *
 got_error_no_obj(struct got_object_id *id)
 {
-	static char msg[sizeof("object   not found") +
-	    SHA1_DIGEST_STRING_LENGTH];
+	char msg[sizeof("object   not found") + SHA1_DIGEST_STRING_LENGTH];
 	char id_str[SHA1_DIGEST_STRING_LENGTH];
 	int ret;
 
@@ -166,7 +179,7 @@ got_error_no_obj(struct got_object_id *id)
 const struct got_error *
 got_error_not_ref(const char *refname)
 {
-	static char msg[sizeof("reference   not found") + 1004];
+	char msg[sizeof("reference   not found") + 1004];
 	int ret;
 
 	ret = snprintf(msg, sizeof(msg), "reference %s not found", refname);
@@ -196,17 +209,17 @@ got_error_uuid(uint32_t uuid_status, const char *prefi
 const struct got_error *
 got_error_path(const char *path, int code)
 {
-	static struct got_error err;
-	static char msg[PATH_MAX + 128];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 	size_t i;
 
 	for (i = 0; i < nitems(got_errors); i++) {
 		if (code == got_errors[i].code) {
-			err.code = code;
-			snprintf(msg, sizeof(msg), "%s: %s", path,
+			err->code = code;
+			snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s", path,
 			    got_errors[i].msg);
-			err.msg = msg;
-			return &err;
+			err->msg = cerr->msg;
+			return err;
 		}
 	}
 
@@ -216,8 +229,8 @@ got_error_path(const char *path, int code)
 const struct got_error *
 got_error_fmt(int code, const char *fmt, ...)
 {
-	static struct got_error err;
-	static char msg[PATH_MAX * 4 + 128];
+	struct got_custom_error *cerr = get_custom_err();
+	struct got_error *err = &cerr->err;
 	char buf[PATH_MAX * 4];
 	va_list ap;
 	size_t i;
@@ -228,11 +241,11 @@ got_error_fmt(int code, const char *fmt, ...)
 
 	for (i = 0; i < nitems(got_errors); i++) {
 		if (code == got_errors[i].code) {
-			err.code = code;
-			snprintf(msg, sizeof(msg), "%s: %s", buf,
+			err->code = code;
+			snprintf(cerr->msg, sizeof(cerr->msg), "%s: %s", buf,
 			    got_errors[i].msg);
-			err.msg = msg;
-			return &err;
+			err->msg = cerr->msg;
+			return err;
 		}
 	}