lib: don't use strerror()

We have and provide Curl_strerror() internally for a reason: strerror()
is not necessarily thread-safe so we should always try to avoid it.

Extended checksrc to warn for this, but feature the check disabled by
default and only enable it in lib/

Closes #7685
This commit is contained in:
Daniel Stenberg 2021-09-08 11:27:49 +02:00
parent 71f9a8fa6f
commit 2f0bb864c1
No known key found for this signature in database
GPG Key ID: 5CC908FDB71E12C2
8 changed files with 71 additions and 22 deletions

1
lib/.checksrc Normal file
View File

@ -0,0 +1 @@
enable STRERROR

View File

@ -47,6 +47,7 @@ my @ignore_line;
my %warnings_extended = ( my %warnings_extended = (
'COPYRIGHTYEAR' => 'copyright year incorrect', 'COPYRIGHTYEAR' => 'copyright year incorrect',
'STRERROR', => 'strerror() detected',
); );
my %warnings = ( my %warnings = (
@ -87,7 +88,7 @@ my %warnings = (
'EXCLAMATIONSPACE' => 'Whitespace after exclamation mark in expression', 'EXCLAMATIONSPACE' => 'Whitespace after exclamation mark in expression',
'EMPTYLINEBRACE' => 'Empty line before the open brace', 'EMPTYLINEBRACE' => 'Empty line before the open brace',
'EQUALSNULL' => 'if/while comparison with == NULL', 'EQUALSNULL' => 'if/while comparison with == NULL',
'NOTEQUALSZERO' => 'if/while comparison with != 0' 'NOTEQUALSZERO', => 'if/while comparison with != 0',
); );
sub readskiplist { sub readskiplist {
@ -238,9 +239,17 @@ if(!$file) {
print " -i<n> Indent spaces. Default: 2\n"; print " -i<n> Indent spaces. Default: 2\n";
print " -m<n> Maximum line length. Default: 79\n"; print " -m<n> Maximum line length. Default: 79\n";
print "\nDetects and warns for these problems:\n"; print "\nDetects and warns for these problems:\n";
for(sort keys %warnings) { my @allw = keys %warnings;
printf (" %-18s: %s\n", $_, $warnings{$_}); push @allw, keys %warnings_extended;
for my $w (sort @allw) {
if($warnings{$w}) {
printf (" %-18s: %s\n", $w, $warnings{$w});
}
else {
printf (" %-18s: %s[*]\n", $w, $warnings_extended{$w});
}
} }
print " [*] = disabled by default\n";
exit; exit;
} }
@ -638,7 +647,18 @@ sub scanfile {
$line, length($1), $file, $ol, $line, length($1), $file, $ol,
"use of $2 is banned"); "use of $2 is banned");
} }
if($warnings{"STRERROR"}) {
# scan for use of banned strerror. This is not a BANNEDFUNC to
# allow for individual enable/disable of this warning.
if($l =~ /^(.*\W)(strerror)\s*\(/x) {
if($1 !~ /^ *\#/) {
# skip preprocessor lines
checkwarn("STRERROR",
$line, length($1), $file, $ol,
"use of $2 is banned");
}
}
}
# scan for use of snprintf for curl-internals reasons # scan for use of snprintf for curl-internals reasons
if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) { if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) {
checkwarn("SNPRINTF", checkwarn("SNPRINTF",

View File

@ -112,11 +112,12 @@ CURLcode Curl_convert_to_network(struct Curl_easy *data,
*cd = iconv_open(CURL_ICONV_CODESET_OF_NETWORK, *cd = iconv_open(CURL_ICONV_CODESET_OF_NETWORK,
CURL_ICONV_CODESET_OF_HOST); CURL_ICONV_CODESET_OF_HOST);
if(*cd == (iconv_t)-1) { if(*cd == (iconv_t)-1) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s", "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
CURL_ICONV_CODESET_OF_NETWORK, CURL_ICONV_CODESET_OF_NETWORK,
CURL_ICONV_CODESET_OF_HOST, CURL_ICONV_CODESET_OF_HOST,
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
} }
@ -128,9 +129,10 @@ CURLcode Curl_convert_to_network(struct Curl_easy *data,
if(!data) if(!data)
iconv_close(tmpcd); iconv_close(tmpcd);
if((rc == ICONV_ERROR) || (in_bytes)) { if((rc == ICONV_ERROR) || (in_bytes)) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"The Curl_convert_to_network iconv call failed with errno %i: %s", "The Curl_convert_to_network iconv call failed with errno %i: %s",
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
#else #else
@ -178,11 +180,12 @@ CURLcode Curl_convert_from_network(struct Curl_easy *data,
*cd = iconv_open(CURL_ICONV_CODESET_OF_HOST, *cd = iconv_open(CURL_ICONV_CODESET_OF_HOST,
CURL_ICONV_CODESET_OF_NETWORK); CURL_ICONV_CODESET_OF_NETWORK);
if(*cd == (iconv_t)-1) { if(*cd == (iconv_t)-1) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s", "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
CURL_ICONV_CODESET_OF_HOST, CURL_ICONV_CODESET_OF_HOST,
CURL_ICONV_CODESET_OF_NETWORK, CURL_ICONV_CODESET_OF_NETWORK,
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
} }
@ -194,9 +197,10 @@ CURLcode Curl_convert_from_network(struct Curl_easy *data,
if(!data) if(!data)
iconv_close(tmpcd); iconv_close(tmpcd);
if((rc == ICONV_ERROR) || (in_bytes)) { if((rc == ICONV_ERROR) || (in_bytes)) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"Curl_convert_from_network iconv call failed with errno %i: %s", "Curl_convert_from_network iconv call failed with errno %i: %s",
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
#else #else
@ -245,11 +249,12 @@ CURLcode Curl_convert_from_utf8(struct Curl_easy *data,
*cd = iconv_open(CURL_ICONV_CODESET_OF_HOST, *cd = iconv_open(CURL_ICONV_CODESET_OF_HOST,
CURL_ICONV_CODESET_FOR_UTF8); CURL_ICONV_CODESET_FOR_UTF8);
if(*cd == (iconv_t)-1) { if(*cd == (iconv_t)-1) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s", "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
CURL_ICONV_CODESET_OF_HOST, CURL_ICONV_CODESET_OF_HOST,
CURL_ICONV_CODESET_FOR_UTF8, CURL_ICONV_CODESET_FOR_UTF8,
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
} }
@ -261,9 +266,10 @@ CURLcode Curl_convert_from_utf8(struct Curl_easy *data,
if(!data) if(!data)
iconv_close(tmpcd); iconv_close(tmpcd);
if((rc == ICONV_ERROR) || (in_bytes)) { if((rc == ICONV_ERROR) || (in_bytes)) {
char buffer[STRERROR_LEN];
failf(data, failf(data,
"The Curl_convert_from_utf8 iconv call failed with errno %i: %s", "The Curl_convert_from_utf8 iconv call failed with errno %i: %s",
errno, strerror(errno)); errno, Curl_strerror(errno, buffer, sizeof(buffer)));
return CURLE_CONV_FAILED; return CURLE_CONV_FAILED;
} }
if(output_ptr < input_ptr) { if(output_ptr < input_ptr) {

View File

@ -731,6 +731,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen)
max = buflen - 1; max = buflen - 1;
*buf = '\0'; *buf = '\0';
/* !checksrc! disable STRERROR 2 */
#if defined(WIN32) || defined(_WIN32_WCE) #if defined(WIN32) || defined(_WIN32_WCE)
#if defined(WIN32) #if defined(WIN32)
/* 'sys_nerr' is the maximum errno number, it is not widely portable */ /* 'sys_nerr' is the maximum errno number, it is not widely portable */

View File

@ -1892,9 +1892,11 @@ static void zonefrom_url(CURLU *uh, struct Curl_easy *data,
#else #else
scopeidx = if_nametoindex(zoneid); scopeidx = if_nametoindex(zoneid);
#endif #endif
if(!scopeidx) if(!scopeidx) {
char buffer[STRERROR_LEN];
infof(data, "Invalid zoneid: %s; %s", zoneid, infof(data, "Invalid zoneid: %s; %s", zoneid,
strerror(errno)); Curl_strerror(errno, buffer, sizeof(buffer)));
}
else else
conn->scope_id = scopeidx; conn->scope_id = scopeidx;
} }

View File

@ -180,6 +180,7 @@ static bool is_separator(char c)
static CURLcode gskit_status(struct Curl_easy *data, int rc, static CURLcode gskit_status(struct Curl_easy *data, int rc,
const char *procname, CURLcode defcode) const char *procname, CURLcode defcode)
{ {
char buffer[STRERROR_LEN];
/* Process GSKit status and map it to a CURLcode. */ /* Process GSKit status and map it to a CURLcode. */
switch(rc) { switch(rc) {
case GSK_OK: case GSK_OK:
@ -208,7 +209,8 @@ static CURLcode gskit_status(struct Curl_easy *data, int rc,
case ENOMEM: case ENOMEM:
return CURLE_OUT_OF_MEMORY; return CURLE_OUT_OF_MEMORY;
default: default:
failf(data, "%s I/O error: %s", procname, strerror(errno)); failf(data, "%s I/O error: %s", procname,
Curl_strerror(errno, buffer, sizeof(buffer)));
break; break;
} }
break; break;
@ -223,13 +225,15 @@ static CURLcode gskit_status(struct Curl_easy *data, int rc,
static CURLcode set_enum(struct Curl_easy *data, gsk_handle h, static CURLcode set_enum(struct Curl_easy *data, gsk_handle h,
GSK_ENUM_ID id, GSK_ENUM_VALUE value, bool unsupported_ok) GSK_ENUM_ID id, GSK_ENUM_VALUE value, bool unsupported_ok)
{ {
char buffer[STRERROR_LEN];
int rc = gsk_attribute_set_enum(h, id, value); int rc = gsk_attribute_set_enum(h, id, value);
switch(rc) { switch(rc) {
case GSK_OK: case GSK_OK:
return CURLE_OK; return CURLE_OK;
case GSK_ERROR_IO: case GSK_ERROR_IO:
failf(data, "gsk_attribute_set_enum() I/O error: %s", strerror(errno)); failf(data, "gsk_attribute_set_enum() I/O error: %s",
Curl_strerror(errno, buffer, sizeof(buffer)));
break; break;
case GSK_ATTRIBUTE_INVALID_ID: case GSK_ATTRIBUTE_INVALID_ID:
if(unsupported_ok) if(unsupported_ok)
@ -245,13 +249,15 @@ static CURLcode set_enum(struct Curl_easy *data, gsk_handle h,
static CURLcode set_buffer(struct Curl_easy *data, gsk_handle h, static CURLcode set_buffer(struct Curl_easy *data, gsk_handle h,
GSK_BUF_ID id, const char *buffer, bool unsupported_ok) GSK_BUF_ID id, const char *buffer, bool unsupported_ok)
{ {
char buffer[STRERROR_LEN];
int rc = gsk_attribute_set_buffer(h, id, buffer, 0); int rc = gsk_attribute_set_buffer(h, id, buffer, 0);
switch(rc) { switch(rc) {
case GSK_OK: case GSK_OK:
return CURLE_OK; return CURLE_OK;
case GSK_ERROR_IO: case GSK_ERROR_IO:
failf(data, "gsk_attribute_set_buffer() I/O error: %s", strerror(errno)); failf(data, "gsk_attribute_set_buffer() I/O error: %s",
Curl_strerror(errno, buffer, sizeof(buffer)));
break; break;
case GSK_ATTRIBUTE_INVALID_ID: case GSK_ATTRIBUTE_INVALID_ID:
if(unsupported_ok) if(unsupported_ok)
@ -267,6 +273,7 @@ static CURLcode set_buffer(struct Curl_easy *data, gsk_handle h,
static CURLcode set_numeric(struct Curl_easy *data, static CURLcode set_numeric(struct Curl_easy *data,
gsk_handle h, GSK_NUM_ID id, int value) gsk_handle h, GSK_NUM_ID id, int value)
{ {
char buffer[STRERROR_LEN];
int rc = gsk_attribute_set_numeric_value(h, id, value); int rc = gsk_attribute_set_numeric_value(h, id, value);
switch(rc) { switch(rc) {
@ -274,7 +281,7 @@ static CURLcode set_numeric(struct Curl_easy *data,
return CURLE_OK; return CURLE_OK;
case GSK_ERROR_IO: case GSK_ERROR_IO:
failf(data, "gsk_attribute_set_numeric_value() I/O error: %s", failf(data, "gsk_attribute_set_numeric_value() I/O error: %s",
strerror(errno)); Curl_strerror(errno, buffer, sizeof(buffer)));
break; break;
default: default:
failf(data, "gsk_attribute_set_numeric_value(): %s", gsk_strerror(rc)); failf(data, "gsk_attribute_set_numeric_value(): %s", gsk_strerror(rc));
@ -287,13 +294,15 @@ static CURLcode set_numeric(struct Curl_easy *data,
static CURLcode set_callback(struct Curl_easy *data, static CURLcode set_callback(struct Curl_easy *data,
gsk_handle h, GSK_CALLBACK_ID id, void *info) gsk_handle h, GSK_CALLBACK_ID id, void *info)
{ {
char buffer[STRERROR_LEN];
int rc = gsk_attribute_set_callback(h, id, info); int rc = gsk_attribute_set_callback(h, id, info);
switch(rc) { switch(rc) {
case GSK_OK: case GSK_OK:
return CURLE_OK; return CURLE_OK;
case GSK_ERROR_IO: case GSK_ERROR_IO:
failf(data, "gsk_attribute_set_callback() I/O error: %s", strerror(errno)); failf(data, "gsk_attribute_set_callback() I/O error: %s",
Curl_strerror(errno, buffer, sizeof(buffer)));
break; break;
default: default:
failf(data, "gsk_attribute_set_callback(): %s", gsk_strerror(rc)); failf(data, "gsk_attribute_set_callback(): %s", gsk_strerror(rc));
@ -966,7 +975,9 @@ static CURLcode gskit_connect_step2(struct Curl_easy *data,
continue; /* Retry. */ continue; /* Retry. */
} }
if(errno != ETIME) { if(errno != ETIME) {
failf(data, "QsoWaitForIOCompletion() I/O error: %s", strerror(errno)); char buffer[STRERROR_LEN];
failf(data, "QsoWaitForIOCompletion() I/O error: %s",
Curl_strerror(errno, buffer, sizeof(buffer)));
cancel_async_handshake(conn, sockindex); cancel_async_handshake(conn, sockindex);
close_async_handshake(connssl); close_async_handshake(connssl);
return CURLE_SSL_CONNECT_ERROR; return CURLE_SSL_CONNECT_ERROR;
@ -1229,7 +1240,8 @@ static int gskit_shutdown(struct Curl_easy *data,
nread = read(conn->sock[sockindex], buf, sizeof(buf)); nread = read(conn->sock[sockindex], buf, sizeof(buf));
if(nread < 0) { if(nread < 0) {
failf(data, "read: %s", strerror(errno)); char buffer[STRERROR_LEN];
failf(data, "read: %s", Curl_strerror(errno, buffer, sizeof(buffer)));
rc = -1; rc = -1;
} }

View File

@ -132,7 +132,9 @@ cr_recv(struct Curl_easy *data, int sockindex,
infof(data, "sread: EAGAIN or EWOULDBLOCK"); infof(data, "sread: EAGAIN or EWOULDBLOCK");
} }
else if(io_error) { else if(io_error) {
failf(data, "reading from socket: %s", strerror(io_error)); char buffer[STRERROR_LEN];
failf(data, "reading from socket: %s",
Curl_strerror(io_error, buffer, sizeof(buffer)));
*err = CURLE_READ_ERROR; *err = CURLE_READ_ERROR;
return -1; return -1;
} }
@ -244,7 +246,9 @@ cr_send(struct Curl_easy *data, int sockindex,
return -1; return -1;
} }
else if(io_error) { else if(io_error) {
failf(data, "writing to socket: %s", strerror(io_error)); char buffer[STRERROR_LEN];
failf(data, "writing to socket: %s",
Curl_strerror(io_error, buffer, sizeof(buffer)));
*err = CURLE_WRITE_ERROR; *err = CURLE_WRITE_ERROR;
return -1; return -1;
} }

View File

@ -34,6 +34,7 @@
#include "multiif.h" #include "multiif.h"
#include "strcase.h" #include "strcase.h"
#include "x509asn1.h" #include "x509asn1.h"
#include "strerror.h"
#ifdef USE_SECTRANSP #ifdef USE_SECTRANSP
@ -3222,7 +3223,9 @@ static int sectransp_shutdown(struct Curl_easy *data,
nread = read(conn->sock[sockindex], buf, sizeof(buf)); nread = read(conn->sock[sockindex], buf, sizeof(buf));
if(nread < 0) { if(nread < 0) {
failf(data, "read: %s", strerror(errno)); char buffer[STRERROR_LEN];
failf(data, "read: %s",
Curl_strerror(errno, buffer, sizeof(buffer)));
rc = -1; rc = -1;
} }