Don't bother sampling __func__.

Removing the function codes continued to sample __func__ for compatibility with
ERR_print_errors_cb, but not ERR_error_string_n. We can just emit
OPENSSL_internal for both. ERR_print_errors_cb already has the file and line
number available which is strictly more information than the function name.
(ERR_error_string_n does not, but we'd already turned that to
OPENSSL_internal.)

This shaves 100kb from a release build of the bssl tool.

In doing so, put an unused function code parameter back into ERR_put_error to
align with OpenSSL. We don't need to pass an additional string in anymore, so
OpenSSL compatibility with anything which uses ERR_LIB_USER or
ERR_get_next_error_library costs nothing. (Not that we need it.)

Change-Id: If6af34628319ade4145190b6f30a0d820e00b20d
Reviewed-on: https://boringssl-review.googlesource.com/6387
Reviewed-by: Adam Langley <agl@google.com>
This commit is contained in:
David Benjamin 2015-10-28 18:03:21 -04:00 committed by Adam Langley
parent 165248c24f
commit 3fc138eccd
6 changed files with 49 additions and 88 deletions

View File

@ -222,7 +222,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) {
ret = setsockopt(bio->num, SOL_SOCKET, SO_KEEPALIVE, (char *)&i,
sizeof(i));
if (ret < 0) {
OPENSSL_PUT_SYSTEM_ERROR(setsockopt);
OPENSSL_PUT_SYSTEM_ERROR();
OPENSSL_PUT_ERROR(BIO, BIO_R_KEEPALIVE);
ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port);
goto exit_loop;
@ -236,7 +236,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) {
c->state = BIO_CONN_S_BLOCKED_CONNECT;
bio->retry_reason = BIO_RR_CONNECT;
} else {
OPENSSL_PUT_SYSTEM_ERROR(connect);
OPENSSL_PUT_SYSTEM_ERROR();
OPENSSL_PUT_ERROR(BIO, BIO_R_CONNECT_ERROR);
ERR_add_error_data(4, "host=", c->param_hostname, ":",
c->param_port);
@ -257,7 +257,7 @@ static int conn_state(BIO *bio, BIO_CONNECT *c) {
ret = -1;
} else {
BIO_clear_retry_flags(bio);
OPENSSL_PUT_SYSTEM_ERROR(connect);
OPENSSL_PUT_SYSTEM_ERROR();
OPENSSL_PUT_ERROR(BIO, BIO_R_NBIO_CONNECT_ERROR);
ERR_add_error_data(4, "host=", c->param_hostname, ":", c->param_port);
ret = 0;

View File

@ -129,7 +129,7 @@ BIO *BIO_new_file(const char *filename, const char *mode) {
file = open_file(filename, mode);
if (file == NULL) {
OPENSSL_PUT_SYSTEM_ERROR(fopen);
OPENSSL_PUT_SYSTEM_ERROR();
ERR_add_error_data(5, "fopen('", filename, "','", mode, "')");
if (errno == ENOENT) {
@ -188,7 +188,7 @@ static int file_read(BIO *b, char *out, int outl) {
size_t ret = fread(out, 1, outl, (FILE *)b->ptr);
if (ret == 0 && ferror((FILE *)b->ptr)) {
OPENSSL_PUT_SYSTEM_ERROR(fread);
OPENSSL_PUT_SYSTEM_ERROR();
OPENSSL_PUT_ERROR(BIO, ERR_R_SYS_LIB);
return -1;
}
@ -258,7 +258,7 @@ static long file_ctrl(BIO *b, int cmd, long num, void *ptr) {
}
fp = open_file(ptr, p);
if (fp == NULL) {
OPENSSL_PUT_SYSTEM_ERROR(fopen);
OPENSSL_PUT_SYSTEM_ERROR();
ERR_add_error_data(5, "fopen('", ptr, "','", p, "')");
OPENSSL_PUT_ERROR(BIO, ERR_R_SYS_LIB);
ret = 0;

View File

@ -68,7 +68,7 @@ int bio_ip_and_port_to_socket_and_addr(int *out_sock,
*out_sock = socket(cur->ai_family, cur->ai_socktype, cur->ai_protocol);
if (*out_sock < 0) {
OPENSSL_PUT_SYSTEM_ERROR(socket);
OPENSSL_PUT_SYSTEM_ERROR();
goto out;
}

View File

@ -281,14 +281,6 @@ uint32_t ERR_peek_error_line_data(const char **file, int *line,
flags);
}
const char *ERR_peek_function(void) {
ERR_STATE *state = err_get_state();
if (state == NULL || state->bottom == state->top) {
return NULL;
}
return state->errors[(state->bottom + 1) % ERR_NUM_ERRORS].function;
}
uint32_t ERR_peek_last_error(void) {
return get_error_values(0 /* peek */, 1 /* top */, NULL, NULL, NULL, NULL);
}
@ -346,8 +338,26 @@ void ERR_clear_system_error(void) {
errno = 0;
}
static void err_error_string(uint32_t packed_error, const char *func_str,
char *buf, size_t len) {
char *ERR_error_string(uint32_t packed_error, char *ret) {
static char buf[ERR_ERROR_STRING_BUF_LEN];
if (ret == NULL) {
/* TODO(fork): remove this. */
ret = buf;
}
#if !defined(NDEBUG)
/* This is aimed to help catch callers who don't provide
* |ERR_ERROR_STRING_BUF_LEN| bytes of space. */
memset(ret, 0, ERR_ERROR_STRING_BUF_LEN);
#endif
ERR_error_string_n(packed_error, ret, ERR_ERROR_STRING_BUF_LEN);
return ret;
}
void 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;
@ -367,17 +377,13 @@ static void err_error_string(uint32_t packed_error, const char *func_str,
lib_str = lib_buf;
}
if (func_str == NULL) {
func_str = "OPENSSL_internal";
}
if (reason_str == NULL) {
BIO_snprintf(reason_buf, sizeof(reason_buf), "reason(%u)", reason);
reason_str = reason_buf;
}
BIO_snprintf(buf, len, "error:%08" PRIx32 ":%s:%s:%s",
packed_error, lib_str, func_str, reason_str);
BIO_snprintf(buf, len, "error:%08" PRIx32 ":%s:OPENSSL_internal:%s",
packed_error, lib_str, reason_str);
if (strlen(buf) == len - 1) {
/* output may be truncated; make sure we always have 5 colon-separated
@ -410,29 +416,6 @@ static void err_error_string(uint32_t packed_error, const char *func_str,
}
}
char *ERR_error_string(uint32_t packed_error, char *ret) {
static char buf[ERR_ERROR_STRING_BUF_LEN];
if (ret == NULL) {
/* TODO(fork): remove this. */
ret = buf;
}
#if !defined(NDEBUG)
/* This is aimed to help catch callers who don't provide
* |ERR_ERROR_STRING_BUF_LEN| bytes of space. */
memset(ret, 0, ERR_ERROR_STRING_BUF_LEN);
#endif
ERR_error_string_n(packed_error, ret, ERR_ERROR_STRING_BUF_LEN);
return ret;
}
void ERR_error_string_n(uint32_t packed_error, char *buf, size_t len) {
err_error_string(packed_error, NULL, buf, len);
}
// err_string_cmp is a compare function for searching error values with
// |bsearch| in |err_string_lookup|.
static int err_string_cmp(const void *a, const void *b) {
@ -461,7 +444,7 @@ static const char *err_string_lookup(uint32_t lib, uint32_t key,
* |6 bits| 11 bits | 15 bits |
*
* The |lib| value is a library identifier: one of the |ERR_LIB_*| values.
* The |key| is either a function or a reason code, depending on the context.
* The |key| is a reason code, depending on the context.
* The |offset| is the number of bytes from the start of |string_data| where
* the (NUL terminated) string for this value can be found.
*
@ -577,13 +560,12 @@ void ERR_print_errors_cb(ERR_print_errors_callback_t callback, void *ctx) {
const unsigned long thread_hash = (uintptr_t) err_get_state();
for (;;) {
const char *function = ERR_peek_function();
packed_error = ERR_get_error_line_data(&file, &line, &data, &flags);
if (packed_error == 0) {
break;
}
err_error_string(packed_error, function, buf, sizeof(buf));
ERR_error_string_n(packed_error, buf, sizeof(buf));
BIO_snprintf(buf2, sizeof(buf2), "%lu:%s:%s:%d:%s\n", thread_hash, buf,
file, line, (flags & ERR_FLAG_STRING) ? data : "");
if (callback(buf2, strlen(buf2), ctx) <= 0) {
@ -623,8 +605,8 @@ static void err_set_error_data(char *data, int flags) {
error->flags = flags;
}
void ERR_put_error(int library, int reason, const char *function,
const char *file, unsigned line) {
void ERR_put_error(int library, int unused, int reason, const char *file,
unsigned line) {
ERR_STATE *const state = err_get_state();
struct err_error_st *error;
@ -647,7 +629,6 @@ void ERR_put_error(int library, int reason, const char *function,
error = &state->errors[state->top];
err_clear(error);
error->function = function;
error->file = file;
error->line = line;
error->packed = ERR_PACK(library, reason);

View File

@ -22,7 +22,7 @@
static bool TestOverflow() {
for (unsigned i = 0; i < ERR_NUM_ERRORS*2; i++) {
ERR_put_error(1, i+1, "function", "test", 1);
ERR_put_error(1, 0 /* unused */, i+1, "test", 1);
}
for (unsigned i = 0; i < ERR_NUM_ERRORS - 1; i++) {
@ -50,7 +50,7 @@ static bool TestPutError() {
return false;
}
ERR_put_error(1, 2, "function", "test", 4);
ERR_put_error(1, 0 /* unused */, 2, "test", 4);
ERR_add_error_data(1, "testing");
int peeked_line, line, peeked_flags, flags;
@ -58,7 +58,6 @@ static bool TestPutError() {
uint32_t peeked_packed_error =
ERR_peek_error_line_data(&peeked_file, &peeked_line, &peeked_data,
&peeked_flags);
const char *function = ERR_peek_function();
uint32_t packed_error = ERR_get_error_line_data(&file, &line, &data, &flags);
if (peeked_packed_error != packed_error ||
@ -69,8 +68,7 @@ static bool TestPutError() {
return false;
}
if (strcmp(function, "function") != 0 ||
strcmp(file, "test") != 0 ||
if (strcmp(file, "test") != 0 ||
line != 4 ||
(flags & ERR_FLAG_STRING) == 0 ||
ERR_GET_LIB(packed_error) != 1 ||
@ -89,7 +87,7 @@ static bool TestClearError() {
return false;
}
ERR_put_error(1, 2, "function", "test", 4);
ERR_put_error(1, 0 /* unused */, 2, "test", 4);
ERR_clear_error();
if (ERR_get_error() != 0) {
@ -101,7 +99,7 @@ static bool TestClearError() {
}
static bool TestPrint() {
ERR_put_error(1, 2, "function", "test", 4);
ERR_put_error(1, 0 /* unused */, 2, "test", 4);
ERR_add_error_data(1, "testing");
uint32_t packed_error = ERR_get_error();
@ -114,7 +112,7 @@ static bool TestPrint() {
}
static bool TestRelease() {
ERR_put_error(1, 2, "function", "test", 4);
ERR_put_error(1, 0 /* unused */, 2, "test", 4);
ERR_remove_thread_state(NULL);
return true;
}
@ -134,11 +132,9 @@ static bool TestPutMacro() {
int line;
const char *file;
const char *function = ERR_peek_function();
uint32_t error = ERR_get_error_line(&file, &line);
if (strcmp(function, "TestPutMacro") != 0 ||
!HasSuffix(file, "err_test.cc") ||
if (!HasSuffix(file, "err_test.cc") ||
line != expected_line ||
ERR_GET_LIB(error) != ERR_LIB_USER ||
ERR_GET_REASON(error) != ERR_R_INTERNAL_ERROR) {

View File

@ -126,7 +126,7 @@ extern "C" {
*
* Each error contains:
* 1) The library (i.e. ec, pem, rsa) which created it.
* 2) The function, file, and line number of the call that added the error.
* 2) The file and line number of the call that added the error.
* 3) A pointer to some error specific data, which may be NULL.
*
* The library identifier and reason code are packed in a uint32_t and there
@ -183,10 +183,6 @@ OPENSSL_EXPORT uint32_t ERR_peek_error_line(const char **file, int *line);
OPENSSL_EXPORT uint32_t ERR_peek_error_line_data(const char **file, int *line,
const char **data, int *flags);
/* ERR_peek_function returns the name of the function which added the least
* recent error or NULL if the queue is empty. */
OPENSSL_EXPORT const char *ERR_peek_function(void);
/* The "peek last" functions act like the "peek" functions, above, except that
* they return the most recent error. */
OPENSSL_EXPORT uint32_t ERR_peek_last_error(void);
@ -246,7 +242,7 @@ typedef int (*ERR_print_errors_callback_t)(const char *str, size_t len,
* The string will have the following format (which differs from
* |ERR_error_string|):
*
* [thread id]:error:[error code]:[library name]:[function name]:
* [thread id]:error:[error code]:[library name]:OPENSSL_internal:
* [reason string]:[file]:[line number]:[optional string data]
*
* (All in one line.)
@ -298,29 +294,20 @@ OPENSSL_EXPORT const char *ERR_func_error_string(uint32_t packed_error);
/* ERR_clear_system_error clears the system's error value (i.e. errno). */
OPENSSL_EXPORT void ERR_clear_system_error(void);
#if defined(OPENSSL_WINDOWS)
/* TODO(davidben): Use |__func__| directly once the minimum MSVC version
* supports it. */
#define OPENSSL_CURRENT_FUNCTION __FUNCTION__
#else
#define OPENSSL_CURRENT_FUNCTION __func__
#endif
/* OPENSSL_PUT_ERROR is used by OpenSSL code to add an error to the error
* queue. */
#define OPENSSL_PUT_ERROR(library, reason) \
ERR_put_error(ERR_LIB_##library, reason, OPENSSL_CURRENT_FUNCTION, __FILE__, \
__LINE__)
ERR_put_error(ERR_LIB_##library, 0, reason, __FILE__, __LINE__)
/* OPENSSL_PUT_SYSTEM_ERROR is used by OpenSSL code to add an error from the
* operating system to the error queue. */
/* TODO(fork): include errno. */
#define OPENSSL_PUT_SYSTEM_ERROR(func) \
ERR_put_error(ERR_LIB_SYS, 0, #func, __FILE__, __LINE__);
#define OPENSSL_PUT_SYSTEM_ERROR() \
ERR_put_error(ERR_LIB_SYS, 0, 0, __FILE__, __LINE__);
/* ERR_put_error adds an error to the error queue, dropping the least recent
* error if neccessary for space reasons. */
OPENSSL_EXPORT void ERR_put_error(int library, int reason, const char *function,
OPENSSL_EXPORT void ERR_put_error(int library, int unused, int reason,
const char *file, unsigned line);
/* ERR_add_error_data takes a variable number (|count|) of const char*
@ -343,15 +330,12 @@ OPENSSL_EXPORT int ERR_set_mark(void);
OPENSSL_EXPORT int ERR_pop_to_mark(void);
struct err_error_st {
/* function contains the name of the function where the error occured. */
const char *function;
/* file contains the filename where the error occured. */
const char *file;
/* data contains optional data. It must be freed with |OPENSSL_free| if
* |flags&ERR_FLAG_MALLOCED|. */
char *data;
/* packed contains the error library, function and reason, as packed by
* ERR_PACK. */
/* packed contains the error library and reason, as packed by ERR_PACK. */
uint32_t packed;
/* line contains the line number where the error occured. */
uint16_t line;