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
2024-11-21 01:16:58 +08:00 · 2021-09-08 11:27:49 +02:00 · 2021-09-08 11:27:49 +02:00 · 2f0bb864c1
commit 2f0bb864c1
parent 71f9a8fa6f
8 changed files with 71 additions and 22 deletions
--- a/lib/.checksrc
+++ b/lib/.checksrc
@ -0,0 +1 @@
+enable STRERROR
--- a/lib/checksrc.pl
+++ b/lib/checksrc.pl
@ -47,6 +47,7 @@ my @ignore_line;

 my %warnings_extended = (
    'COPYRIGHTYEAR'    => 'copyright year incorrect',
+    'STRERROR',        => 'strerror() detected',
    );

 my %warnings = (
@ -87,7 +88,7 @@ my %warnings = (
    'EXCLAMATIONSPACE' => 'Whitespace after exclamation mark in expression',
    'EMPTYLINEBRACE'   => 'Empty line before the open brace',
    'EQUALSNULL'       => 'if/while comparison with == NULL',
-    'NOTEQUALSZERO'    => 'if/while comparison with != 0'
+    'NOTEQUALSZERO',   => 'if/while comparison with != 0',
    );

 sub readskiplist {
@ -238,9 +239,17 @@ if(!$file) {
    print "  -i<n>     Indent spaces. Default: 2\n";
    print "  -m<n>     Maximum line length. Default: 79\n";
    print "\nDetects and warns for these problems:\n";
-    for(sort keys %warnings) {
-        printf (" %-18s: %s\n", $_, $warnings{$_});
+    my @allw = keys %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;
 }

@ -638,7 +647,18 @@ sub scanfile {
                      $line, length($1), $file, $ol,
                      "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
        if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) {
            checkwarn("SNPRINTF",
--- a/lib/non-ascii.c
+++ b/lib/non-ascii.c
@ -112,11 +112,12 @@ CURLcode Curl_convert_to_network(struct Curl_easy *data,
      *cd = iconv_open(CURL_ICONV_CODESET_OF_NETWORK,
                       CURL_ICONV_CODESET_OF_HOST);
      if(*cd == (iconv_t)-1) {
+        char buffer[STRERROR_LEN];
        failf(data,
              "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
              CURL_ICONV_CODESET_OF_NETWORK,
              CURL_ICONV_CODESET_OF_HOST,
-              errno, strerror(errno));
+              errno, Curl_strerror(errno, buffer, sizeof(buffer)));
        return CURLE_CONV_FAILED;
      }
    }
@ -128,9 +129,10 @@ CURLcode Curl_convert_to_network(struct Curl_easy *data,
    if(!data)
      iconv_close(tmpcd);
    if((rc == ICONV_ERROR) || (in_bytes)) {
+      char buffer[STRERROR_LEN];
      failf(data,
            "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;
    }
 #else
@ -178,11 +180,12 @@ CURLcode Curl_convert_from_network(struct Curl_easy *data,
      *cd = iconv_open(CURL_ICONV_CODESET_OF_HOST,
                       CURL_ICONV_CODESET_OF_NETWORK);
      if(*cd == (iconv_t)-1) {
+        char buffer[STRERROR_LEN];
        failf(data,
              "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
              CURL_ICONV_CODESET_OF_HOST,
              CURL_ICONV_CODESET_OF_NETWORK,
-              errno, strerror(errno));
+              errno, Curl_strerror(errno, buffer, sizeof(buffer)));
        return CURLE_CONV_FAILED;
      }
    }
@ -194,9 +197,10 @@ CURLcode Curl_convert_from_network(struct Curl_easy *data,
    if(!data)
      iconv_close(tmpcd);
    if((rc == ICONV_ERROR) || (in_bytes)) {
+      char buffer[STRERROR_LEN];
      failf(data,
            "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;
    }
 #else
@ -245,11 +249,12 @@ CURLcode Curl_convert_from_utf8(struct Curl_easy *data,
      *cd = iconv_open(CURL_ICONV_CODESET_OF_HOST,
                       CURL_ICONV_CODESET_FOR_UTF8);
      if(*cd == (iconv_t)-1) {
+        char buffer[STRERROR_LEN];
        failf(data,
              "The iconv_open(\"%s\", \"%s\") call failed with errno %i: %s",
              CURL_ICONV_CODESET_OF_HOST,
              CURL_ICONV_CODESET_FOR_UTF8,
-              errno, strerror(errno));
+              errno, Curl_strerror(errno, buffer, sizeof(buffer)));
        return CURLE_CONV_FAILED;
      }
    }
@ -261,9 +266,10 @@ CURLcode Curl_convert_from_utf8(struct Curl_easy *data,
    if(!data)
      iconv_close(tmpcd);
    if((rc == ICONV_ERROR) || (in_bytes)) {
+      char buffer[STRERROR_LEN];
      failf(data,
            "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;
    }
    if(output_ptr < input_ptr) {
--- a/lib/strerror.c
+++ b/lib/strerror.c
@ -731,6 +731,7 @@ const char *Curl_strerror(int err, char *buf, size_t buflen)
  max = buflen - 1;
  *buf = '\0';

+  /* !checksrc! disable STRERROR 2 */
 #if defined(WIN32) || defined(_WIN32_WCE)
 #if defined(WIN32)
  /* 'sys_nerr' is the maximum errno number, it is not widely portable */
--- a/lib/url.c
+++ b/lib/url.c
@ -1892,9 +1892,11 @@ static void zonefrom_url(CURLU *uh, struct Curl_easy *data,
 #else
      scopeidx = if_nametoindex(zoneid);
 #endif
-      if(!scopeidx)
+      if(!scopeidx) {
+        char buffer[STRERROR_LEN];
        infof(data, "Invalid zoneid: %s; %s", zoneid,
-              strerror(errno));
+              Curl_strerror(errno, buffer, sizeof(buffer)));
+      }
      else
        conn->scope_id = scopeidx;
    }
--- a/lib/vtls/gskit.c
+++ b/lib/vtls/gskit.c
@ -180,6 +180,7 @@ static bool is_separator(char c)
 static CURLcode gskit_status(struct Curl_easy *data, int rc,
                             const char *procname, CURLcode defcode)
 {
+  char buffer[STRERROR_LEN];
  /* Process GSKit status and map it to a CURLcode. */
  switch(rc) {
  case GSK_OK:
@ -208,7 +209,8 @@ static CURLcode gskit_status(struct Curl_easy *data, int rc,
    case ENOMEM:
      return CURLE_OUT_OF_MEMORY;
    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;
@ -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,
                GSK_ENUM_ID id, GSK_ENUM_VALUE value, bool unsupported_ok)
 {
+  char buffer[STRERROR_LEN];
  int rc = gsk_attribute_set_enum(h, id, value);

  switch(rc) {
  case GSK_OK:
    return CURLE_OK;
  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;
  case GSK_ATTRIBUTE_INVALID_ID:
    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,
                        GSK_BUF_ID id, const char *buffer, bool unsupported_ok)
 {
+  char buffer[STRERROR_LEN];
  int rc = gsk_attribute_set_buffer(h, id, buffer, 0);

  switch(rc) {
  case GSK_OK:
    return CURLE_OK;
  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;
  case GSK_ATTRIBUTE_INVALID_ID:
    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,
                            gsk_handle h, GSK_NUM_ID id, int value)
 {
+  char buffer[STRERROR_LEN];
  int rc = gsk_attribute_set_numeric_value(h, id, value);

  switch(rc) {
@ -274,7 +281,7 @@ static CURLcode set_numeric(struct Curl_easy *data,
    return CURLE_OK;
  case GSK_ERROR_IO:
    failf(data, "gsk_attribute_set_numeric_value() I/O error: %s",
-          strerror(errno));
+          Curl_strerror(errno, buffer, sizeof(buffer)));
    break;
  default:
    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,
                             gsk_handle h, GSK_CALLBACK_ID id, void *info)
 {
+  char buffer[STRERROR_LEN];
  int rc = gsk_attribute_set_callback(h, id, info);

  switch(rc) {
  case GSK_OK:
    return CURLE_OK;
  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;
  default:
    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. */
      }
      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);
        close_async_handshake(connssl);
        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));

    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;
    }

--- a/lib/vtls/rustls.c
+++ b/lib/vtls/rustls.c
@ -132,7 +132,9 @@ cr_recv(struct Curl_easy *data, int sockindex,
    infof(data, "sread: EAGAIN or EWOULDBLOCK");
  }
  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;
    return -1;
  }
@ -244,7 +246,9 @@ cr_send(struct Curl_easy *data, int sockindex,
      return -1;
    }
    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;
      return -1;
    }
--- a/lib/vtls/sectransp.c
+++ b/lib/vtls/sectransp.c
@ -34,6 +34,7 @@
 #include "multiif.h"
 #include "strcase.h"
 #include "x509asn1.h"
+#include "strerror.h"

 #ifdef USE_SECTRANSP

@ -3222,7 +3223,9 @@ static int sectransp_shutdown(struct Curl_easy *data,
    nread = read(conn->sock[sockindex], buf, sizeof(buf));

    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;
    }