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>

crypto/err/err.c

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

include/openssl/err.h

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.
 //