Give ERR_error_string_n a return value for convenience.
ERR_error_string_n needs to be called in a separate statement, compared
to ERR_error_string(err, NULL), which returns a buffer and is very
convenient to use in an expression. This is unfortunate because it is
not thread-safe.
Give ERR_error_string_n a return value to align. Fixing callers still
requires allocating a buffer somewhere, but the rest of the expression
can remain relatively unperturbed.
Change-Id: I273c9df97f0bb113cdc57cf3896c42195910c67a
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/38964
Reviewed-by: Steven Valdez <svaldez@google.com>
Commit-Queue: David Benjamin <davidben@google.com>
diff --git a/crypto/err/err.c b/crypto/err/err.c
index 43d3909..a432ce3 100644
--- a/crypto/err/err.c
+++ b/crypto/err/err.c
@@ -382,18 +382,16 @@
OPENSSL_memset(ret, 0, ERR_ERROR_STRING_BUF_LEN);
#endif
- ERR_error_string_n(packed_error, ret, ERR_ERROR_STRING_BUF_LEN);
-
- return ret;
+ return ERR_error_string_n(packed_error, ret, ERR_ERROR_STRING_BUF_LEN);
}
-void ERR_error_string_n(uint32_t packed_error, char *buf, size_t len) {
+char *ERR_error_string_n(uint32_t packed_error, char *buf, size_t len) {
char lib_buf[64], reason_buf[64];
const char *lib_str, *reason_str;
unsigned lib, reason;
if (len == 0) {
- return;
+ return NULL;
}
lib = ERR_GET_LIB(packed_error);
@@ -425,7 +423,7 @@
if (len <= num_colons) {
// In this situation it's not possible to ensure that the correct number
// of colons are included in the output.
- return;
+ return buf;
}
for (i = 0; i < num_colons; i++) {
@@ -444,6 +442,8 @@
s = colon + 1;
}
}
+
+ return buf;
}
// err_string_cmp is a compare function for searching error values with
diff --git a/include/openssl/err.h b/include/openssl/err.h
index 4721f75..f9cd9f2 100644
--- a/include/openssl/err.h
+++ b/include/openssl/err.h
@@ -209,9 +209,9 @@
int *flags);
// ERR_error_string_n generates a human-readable string representing
-// |packed_error| and places it at |buf|. It writes at most |len| bytes
-// (including the terminating NUL) and truncates the string if necessary. If
-// |len| is greater than zero then |buf| is always NUL terminated.
+// |packed_error|, places it at |buf|, and returns |buf|. It writes at most
+// |len| bytes (including the terminating NUL) and truncates the string if
+// necessary. If |len| is greater than zero then |buf| is always NUL terminated.
//
// The string will have the following format:
//
@@ -219,8 +219,8 @@
//
// error code is an 8 digit hexadecimal number; library name and reason string
// are ASCII text.
-OPENSSL_EXPORT void ERR_error_string_n(uint32_t packed_error, char *buf,
- size_t len);
+OPENSSL_EXPORT char *ERR_error_string_n(uint32_t packed_error, char *buf,
+ size_t len);
// ERR_lib_error_string returns a string representation of the library that
// generated |packed_error|.
@@ -389,10 +389,12 @@
OPENSSL_EXPORT const char *ERR_func_error_string(uint32_t packed_error);
// ERR_error_string behaves like |ERR_error_string_n| but |len| is implicitly
-// |ERR_ERROR_STRING_BUF_LEN| and it returns |buf|. If |buf| is NULL, the error
-// string is placed in a static buffer which is returned. (The static buffer may
-// be overridden by concurrent calls in other threads so this form should not be
-// used.)
+// |ERR_ERROR_STRING_BUF_LEN|.
+//
+// Additionally, if |buf| is NULL, the error string is placed in a static buffer
+// which is returned. This is not thread-safe and only exists for backwards
+// compatibility with legacy callers. The static buffer will be overridden by
+// calls in other threads.
//
// Use |ERR_error_string_n| instead.
//
