Skip to content

Commit

Permalink
Avoid strdup in crypto/err/err.c
Browse files Browse the repository at this point in the history
This makes me sad, but strdup may be more trouble than is worth it?
Being not in C (until C23) and only a (by POSIX standards) recent
addition to POSIX means a lot of folks seem to make it unnecessarily
hard to use:

- MSVC adds a deprecation warning that we have to suppress

- glibc gates it on feature macros; we just don't notice because we
  already have to work around their bad behavior for pthread_rwlock

- musl gates it on feature macros, which was one of the things that
  tripped cl/583161936

Given we only want to use strdup in one file (err.c, which wants to
avoid OPENSSL_malloc), a small reimplementation is probably not the end
of the world.

While I'm here, we can actually make OPENSSL_strdup's implementation a
little simpler.

Change-Id: I4e6c743b3104a67357d7d527c178c615de6bc844
Reviewed-on: https://boringssl-review.googlesource.com/c/boringssl/+/64047
Reviewed-by: Bob Beck <[email protected]>
Commit-Queue: David Benjamin <[email protected]>
(cherry picked from commit 89f097740e6376521926eb56a61b25f639c473ac)
  • Loading branch information
davidben authored and justsmth committed Jan 17, 2025
1 parent a41b35d commit f4a4864
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 17 deletions.
23 changes: 13 additions & 10 deletions crypto/err/err.c
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,17 @@ extern const uint32_t kOpenSSLReasonValues[];
extern const size_t kOpenSSLReasonValuesLen;
extern const char kOpenSSLReasonStringData[];

static char *strdup_libc_malloc(const char *str) {
// |strdup| is not in C until C23, so MSVC triggers deprecation warnings, and
// glibc and musl gate it on a feature macro. Reimplementing it is easier.
size_t len = strlen(str);
char *ret = malloc(len + 1);
if (ret != NULL) {
memcpy(ret, str, len + 1);
}
return ret;
}

// err_clear clears the given queued error.
static void err_clear(struct err_error_st *error) {
free(error->data);
Expand All @@ -188,13 +199,9 @@ static void err_copy(struct err_error_st *dst, const struct err_error_st *src) {
err_clear(dst);
dst->file = src->file;
if (src->data != NULL) {
// Disable deprecated functions on msvc so it doesn't complain about strdup.
OPENSSL_MSVC_PRAGMA(warning(push))
OPENSSL_MSVC_PRAGMA(warning(disable : 4996))
// We can't use OPENSSL_strdup because we don't want to call OPENSSL_malloc,
// which can affect the error stack.
dst->data = strdup(src->data);
OPENSSL_MSVC_PRAGMA(warning(pop))
dst->data = strdup_libc_malloc(src->data);
}
dst->packed = src->packed;
dst->line = src->line;
Expand Down Expand Up @@ -766,13 +773,9 @@ void ERR_set_error_data(char *data, int flags) {
assert(0);
return;
}
// Disable deprecated functions on msvc so it doesn't complain about strdup.
OPENSSL_MSVC_PRAGMA(warning(push))
OPENSSL_MSVC_PRAGMA(warning(disable : 4996))
// We can not use OPENSSL_strdup because we don't want to call OPENSSL_malloc,
// which can affect the error stack.
char *copy = strdup(data);
OPENSSL_MSVC_PRAGMA(warning(pop))
char *copy = strdup_libc_malloc(data);
if (copy != NULL) {
err_set_error_data(copy);
}
Expand Down
9 changes: 2 additions & 7 deletions crypto/mem.c
Original file line number Diff line number Diff line change
Expand Up @@ -379,13 +379,8 @@ char *OPENSSL_strdup(const char *s) {
if (s == NULL) {
return NULL;
}
const size_t len = strlen(s) + 1;
char *ret = OPENSSL_malloc(len);
if (ret == NULL) {
return NULL;
}
OPENSSL_memcpy(ret, s, len);
return ret;
// Copy the NUL terminator.
return OPENSSL_memdup(s, strlen(s) + 1);
}

int OPENSSL_isalpha(int c) {
Expand Down

0 comments on commit f4a4864

Please sign in to comment.