tool: use our own stderr variable

Earlier this year we changed our own stderr variable to use the standard
name `stderr` (to avoid bugs where someone is using `stderr` instead of
the curl-tool specific variable). This solution needed to override the
standard `stderr` symbol via the preprocessor. This in turn didn't play
well with unity builds and caused curl tool to crash or stay silent due
to an uninitialized stderr. This was a hard to find issue, fixed by
manually breaking out one file from the unity sources.

To avoid two these two tricks, this patch implements a different
solution: Restore using our own local variable for our stderr output and
leave `stderr` as-is. To avoid using `stderr` by mistake, add a
`checksrc` rule (based on logic we already used in lib for `strerror`)
that detects any `stderr` use in `src` and points to using our own
variable instead: `tool_stderr`.

Follow-up to 06133d3e9b8aeb9e9ca0b3370c246bdfbfc8619e
Follow-up to 2f17a9b654121dd1ecf4fc043c6d08a9da3522db

Closes #11958

diff --git a/.reuse/dep5 b/.reuse/dep5
index cdefb1698f06676321fdd280ba3b2d8e66fc2585..3a7a672a2b68976ab50306a0ec6d989716254042 100644 (file)
--- a/.reuse/dep5
+++ b/.reuse/dep5
@@ -41,7 +41,7 @@ Copyright: Daniel Stenberg, <daniel@haxx.se>, et al.
 License: curl
 
 # checksrc control files
-Files: lib/.checksrc docs/examples/.checksrc tests/libtest/.checksrc
+Files: lib/.checksrc src/.checksrc docs/examples/.checksrc tests/libtest/.checksrc
 Copyright: Daniel Stenberg, <daniel@haxx.se>, et al.
 License: curl
 

diff --git a/docs/CHECKSRC.md b/docs/CHECKSRC.md
index 476df262a1ad54936f7077c95a38e58aa638f93a..f73e7a8347a4f77f923be56c60f63e4bb94745ad 100644 (file)
--- a/docs/CHECKSRC.md
+++ b/docs/CHECKSRC.md
@@ -137,6 +137,8 @@ Currently these are the extended warnings which can be enabled:
 
 - `STRERROR`: use of banned function strerror()
 
+- `STDERR`: use of banned variable `stderr`
+
 ## Ignore certain warnings
 
 Due to the nature of the source code and the flaws of the `checksrc` tool,

diff --git a/scripts/checksrc.pl b/scripts/checksrc.pl
index b08b4ffc8b1dee8bd706f659bca7d7b75100a10e..b85b56b4318deed213ba9fe6249f2dde6d7cb350 100755 (executable)
--- a/scripts/checksrc.pl
+++ b/scripts/checksrc.pl
@@ -50,6 +50,7 @@ my @ignore_line;
 my %warnings_extended = (
     'COPYRIGHTYEAR'    => 'copyright year incorrect',
     'STRERROR',        => 'strerror() detected',
+    'STDERR',          => 'stderr detected',
     );
 
 my %warnings = (
@@ -729,6 +730,18 @@ sub scanfile {
                 }
             }
         }
+        if($warnings{"STDERR"}) {
+            # scan for use of banned stderr. This is not a BANNEDFUNC to
+            # allow for individual enable/disable of this warning.
+            if($l =~ /^([^\"-]*\W)(stderr)[^\"_]/x) {
+                if($1 !~ /^ *\#/) {
+                    # skip preprocessor lines
+                    checkwarn("STDERR",
+                              $line, length($1), $file, $ol,
+                              "use of $2 is banned (use tool_stderr instead)");
+                }
+            }
+        }
         # scan for use of snprintf for curl-internals reasons
         if($l =~ /^(.*\W)(v?snprintf)\s*\(/x) {
             checkwarn("SNPRINTF",

diff --git a/src/.checksrc b/src/.checksrc
new file mode 100644 (file)
index 0000000..946367c
--- /dev/null
+++ b/src/.checksrc
@@ -0,0 +1 @@
+enable STDERR

diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt
index 08a3e5c4746118d57d89b8b947ad5912f526ed9b..3d6334b2cfa3bccbd97127777bdee52dabde4a5d 100644 (file)
--- a/src/CMakeLists.txt
+++ b/src/CMakeLists.txt
@@ -67,11 +67,6 @@ if(BUILD_STATIC_CURL)
   set(CURLX_CFILES ${CURLTOOL_LIBCURL_CFILES})
 endif()
 
-# Compile this source separately to avoid having `stderr` overridden in this
-# file. Necessary for successfully initializing stderr in unity builds.
-# (Not necessary for the libcurltool target.)
-set_source_files_properties(tool_stderr.c PROPERTIES SKIP_UNITY_BUILD_INCLUSION ON)
-
 add_executable(
   ${EXE_NAME}
   ${CURL_CFILES} ${CURLX_CFILES} ${CURL_HFILES}

diff --git a/src/tool_cb_dbg.c b/src/tool_cb_dbg.c
index d53a43978dca42843837f74f7fbf6cc8e66822cc..ce5e25e92c353f76535b00d33972d5f936ed723d 100644 (file)
--- a/src/tool_cb_dbg.c
+++ b/src/tool_cb_dbg.c
@@ -95,7 +95,7 @@ int tool_debug_cb(CURL *handle, curl_infotype type,
 {
   struct OperationConfig *operation = userdata;
   struct GlobalConfig *config = operation->global;
-  FILE *output = stderr;
+  FILE *output = tool_stderr;
   const char *text;
   struct timeval tv;
   char timebuf[20];
@@ -137,7 +137,7 @@ int tool_debug_cb(CURL *handle, curl_infotype type,
       config->trace_stream = stdout;
     else if(!strcmp("%", config->trace_dump))
       /* Ok, this is somewhat hackish but we do it undocumented for now */
-      config->trace_stream = stderr;
+      config->trace_stream = tool_stderr;
     else {
       config->trace_stream = fopen(config->trace_dump, FOPEN_WRITETEXT);
       config->trace_fopened = TRUE;
@@ -195,7 +195,8 @@ int tool_debug_cb(CURL *handle, curl_infotype type,
            to stderr or stdout, we don't display the alert about the data not
            being shown as the data _is_ shown then just not via this
            function */
-        if(!config->isatty || ((output != stderr) && (output != stdout))) {
+        if(!config->isatty ||
+           ((output != tool_stderr) && (output != stdout))) {
           if(!newl)
             log_line_start(output, timebuf, idsbuf, type);
           fprintf(output, "[%zu bytes data]\n", size);

diff --git a/src/tool_cb_prg.c b/src/tool_cb_prg.c
index 9c8ffb2b5aef143ace5e342d81d8c55ab42b611c..47ec3ef3977d1f504af40ab4707ea450ab96d42c 100644 (file)
--- a/src/tool_cb_prg.c
+++ b/src/tool_cb_prg.c
@@ -274,7 +274,7 @@ void progressbarinit(struct ProgressData *bar,
   else if(bar->width > MAX_BARLENGTH)
     bar->width = MAX_BARLENGTH;
 
-  bar->out = stderr;
+  bar->out = tool_stderr;
   bar->tick = 150;
   bar->barmove = 1;
 }

diff --git a/src/tool_getparam.c b/src/tool_getparam.c
index a397ad575ea054c7f727608952b2d338c97a3d2c..cd9ce7b5adc7da7ed8f4f9bb42cbd9314a12e680 100644 (file)
--- a/src/tool_getparam.c
+++ b/src/tool_getparam.c
@@ -2777,9 +2777,9 @@ ParameterError parse_args(struct GlobalConfig *global, int argc,
     const char *reason = param2text(result);
 
     if(orig_opt && strcmp(":", orig_opt))
-      helpf(stderr, "option %s: %s", orig_opt, reason);
+      helpf(tool_stderr, "option %s: %s", orig_opt, reason);
     else
-      helpf(stderr, "%s", reason);
+      helpf(tool_stderr, "%s", reason);
   }
 
   curlx_unicodefree(orig_opt);

diff --git a/src/tool_getpass.c b/src/tool_getpass.c
index a4fb35108d0d165ab90030ebcac917dbfbb72310..f5aa98c6adfe720975efe7b7533f3c1b468d68af 100644 (file)
--- a/src/tool_getpass.c
+++ b/src/tool_getpass.c
@@ -99,7 +99,7 @@ char *getpass_r(const char *prompt, char *buffer, size_t buflen)
 char *getpass_r(const char *prompt, char *buffer, size_t buflen)
 {
   size_t i;
-  fputs(prompt, stderr);
+  fputs(prompt, tool_stderr);
 
   for(i = 0; i < buflen; i++) {
     buffer[i] = (char)getch();
@@ -114,7 +114,7 @@ char *getpass_r(const char *prompt, char *buffer, size_t buflen)
         i = i - (i >= 1 ? 2 : 1);
   }
   /* since echo is disabled, print a newline */
-  fputs("\n", stderr);
+  fputs("\n", tool_stderr);
   /* if user didn't hit ENTER, terminate buffer */
   if(i == buflen)
     buffer[buflen-1] = '\0';
@@ -184,7 +184,7 @@ char *getpass_r(const char *prompt, /* prompt to display */
 
   disabled = ttyecho(FALSE, fd); /* disable terminal echo */
 
-  fputs(prompt, stderr);
+  fputs(prompt, tool_stderr);
   nread = read(fd, password, buflen);
   if(nread > 0)
     password[--nread] = '\0'; /* null-terminate where enter is stored */
@@ -193,7 +193,7 @@ char *getpass_r(const char *prompt, /* prompt to display */
 
   if(disabled) {
     /* if echo actually was disabled, add a newline */
-    fputs("\n", stderr);
+    fputs("\n", tool_stderr);
     (void)ttyecho(TRUE, fd); /* enable echo */
   }
 

diff --git a/src/tool_help.c b/src/tool_help.c
index 729b78c366e1b9fc35b52c10d4ebf0e95b4dc2b9..8983a4ca0c6dd1c1dbccea21b3178bf514ece173 100644 (file)
--- a/src/tool_help.c
+++ b/src/tool_help.c
@@ -164,7 +164,7 @@ void tool_version_info(void)
 {
   const char *const *builtin;
   if(is_debug())
-    fprintf(stderr, "WARNING: this libcurl is Debug-enabled, "
+    fprintf(tool_stderr, "WARNING: this libcurl is Debug-enabled, "
             "do not use in production\n\n");
 
   printf(CURL_ID "%s\n", curl_version());

diff --git a/src/tool_main.h b/src/tool_main.h
index cae520efbe5ddd7445f4745f81179cd42dbfdc35..c4da177e1150eb648f1fdd819a1e00496ad0adbe 100644 (file)
--- a/src/tool_main.h
+++ b/src/tool_main.h
@@ -42,7 +42,7 @@
 #endif
 
 #ifndef STDERR_FILENO
-#  define STDERR_FILENO  fileno(stderr)
+#  define STDERR_FILENO  fileno(tool_stderr)
 #endif
 
 #endif /* HEADER_CURL_TOOL_MAIN_H */

diff --git a/src/tool_msgs.c b/src/tool_msgs.c
index 9754870a39a2398fea22857c00675780854c3ddb..c914836db77ebd7845a542a9515ba405a718848e 100644 (file)
--- a/src/tool_msgs.c
+++ b/src/tool_msgs.c
@@ -55,7 +55,7 @@ static void voutf(struct GlobalConfig *config,
 
     ptr = print_buffer;
     while(len > 0) {
-      fputs(prefix, stderr);
+      fputs(prefix, tool_stderr);
 
       if(len > width) {
         size_t cut = width-1;
@@ -68,14 +68,14 @@ static void voutf(struct GlobalConfig *config,
              max text width then! */
           cut = width-1;
 
-        (void)fwrite(ptr, cut + 1, 1, stderr);
-        fputs("\n", stderr);
+        (void)fwrite(ptr, cut + 1, 1, tool_stderr);
+        fputs("\n", tool_stderr);
         ptr += cut + 1; /* skip the space too */
         len -= cut + 1;
       }
       else {
-        fputs(ptr, stderr);
-        fputs("\n", stderr);
+        fputs(ptr, tool_stderr);
+        fputs("\n", tool_stderr);
         len = 0;
       }
     }

diff --git a/src/tool_operate.c b/src/tool_operate.c
index 388b61f52e79a61db03bccfb3afc41fd62a37fb9..4991186ebf05b89a96ed8604eeece83758a99eeb 100644 (file)
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -304,7 +304,7 @@ static CURLcode pre_transfer(struct GlobalConfig *global,
     if((per->infd == -1) || fstat(per->infd, &fileinfo))
 #endif
     {
-      helpf(stderr, "Can't open '%s'", per->uploadfile);
+      helpf(tool_stderr, "Can't open '%s'", per->uploadfile);
       if(per->infd != -1) {
         close(per->infd);
         per->infd = STDIN_FILENO;
@@ -415,10 +415,10 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
     if(!config->synthetic_error && result &&
        (!global->silent || global->showerror)) {
       const char *msg = per->errorbuffer;
-      fprintf(stderr, "curl: (%d) %s\n", result,
+      fprintf(tool_stderr, "curl: (%d) %s\n", result,
               (msg && msg[0]) ? msg : curl_easy_strerror(result));
       if(result == CURLE_PEER_FAILED_VERIFICATION)
-        fputs(CURL_CA_CERT_ERRORMSG, stderr);
+        fputs(CURL_CA_CERT_ERRORMSG, tool_stderr);
     }
     else if(config->failwithbody) {
       /* if HTTP response >= 400, return error */
@@ -426,7 +426,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
       curl_easy_getinfo(curl, CURLINFO_RESPONSE_CODE, &code);
       if(code >= 400) {
         if(!global->silent || global->showerror)
-          fprintf(stderr,
+          fprintf(tool_stderr,
                   "curl: (%d) The requested URL returned error: %ld\n",
                   CURLE_HTTP_RETURNED_ERROR, code);
         result = CURLE_HTTP_RETURNED_ERROR;
@@ -864,19 +864,19 @@ clean:
 
   switch(result) {
   case CURLE_URL_MALFORMAT:
-    helpf(stderr, "malformed URL. Visit https://curl.se/"
+    helpf(tool_stderr, "malformed URL. Visit https://curl.se/"
           "docs/ipfs.html#Gateway-file-and-"
           "environment-variable for more "
           "information");
     break;
   case CURLE_FILE_COULDNT_READ_FILE:
-    helpf(stderr, "IPFS automatic gateway detection "
+    helpf(tool_stderr, "IPFS automatic gateway detection "
           "failure. Visit https://curl.se/docs/"
           "ipfs.html#Malformed-gateway-URL for "
           "more information");
     break;
   case CURLE_BAD_FUNCTION_ARGUMENT:
-    helpf(stderr, "--ipfs-gateway argument results in "
+    helpf(tool_stderr, "--ipfs-gateway argument results in "
           "malformed URL. Visit https://curl.se/"
           "docs/ipfs.html#Malformed-gateway-URL "
           "for more information");
@@ -1020,7 +1020,7 @@ static CURLcode single_transfer(struct GlobalConfig *global,
       /* Unless explicitly shut off */
       result = glob_url(&inglob, infiles, &state->infilenum,
                         (!global->silent || global->showerror)?
-                        stderr:NULL);
+                        tool_stderr:NULL);
       if(result)
         break;
       config->state.inglob = inglob;
@@ -1056,7 +1056,7 @@ static CURLcode single_transfer(struct GlobalConfig *global,
              expressions and return total number of URLs in pattern set */
           result = glob_url(&state->urls, urlnode->url, &state->urlnum,
                             (!global->silent || global->showerror)?
-                            stderr:NULL);
+                            tool_stderr:NULL);
           if(result)
             break;
           urlnum = state->urlnum;
@@ -2074,7 +2074,7 @@ static CURLcode single_transfer(struct GlobalConfig *global,
         my_setopt(curl, CURLOPT_TIMEVALUE_LARGE, config->condtime);
         my_setopt_str(curl, CURLOPT_CUSTOMREQUEST, config->customrequest);
         customrequest_helper(config, config->httpreq, config->customrequest);
-        my_setopt(curl, CURLOPT_STDERR, stderr);
+        my_setopt(curl, CURLOPT_STDERR, tool_stderr);
 
         /* three new ones in libcurl 7.3: */
         my_setopt_str(curl, CURLOPT_INTERFACE, config->iface);
@@ -2745,7 +2745,7 @@ static CURLcode transfer_per_config(struct GlobalConfig *global,
 
   /* Check we have a url */
   if(!config->url_list || !config->url_list->url) {
-    helpf(stderr, "(%d) no URL specified", CURLE_FAILED_INIT);
+    helpf(tool_stderr, "(%d) no URL specified", CURLE_FAILED_INIT);
     return CURLE_FAILED_INIT;
   }
 
@@ -2922,7 +2922,7 @@ CURLcode operate(struct GlobalConfig *global, int argc, argv_item_t argv[])
 
     /* If we had no arguments then make sure a url was specified in .curlrc */
     if((argc < 2) && (!global->first->url_list)) {
-      helpf(stderr, NULL);
+      helpf(tool_stderr, NULL);
       result = CURLE_FAILED_INIT;
     }
   }

diff --git a/src/tool_parsecfg.c b/src/tool_parsecfg.c
index 0bc69646b09d3d0d707f248639382deb3df8654b..c15f2104353081f12ca2406c2e79029f3a934cd3 100644 (file)
--- a/src/tool_parsecfg.c
+++ b/src/tool_parsecfg.c
@@ -168,7 +168,7 @@ int parseconfig(const char *filename, struct GlobalConfig *global)
         *line++ = '\0'; /* null-terminate, we have a local copy of the data */
 
 #ifdef DEBUG_CONFIG
-      fprintf(stderr, "GOT: %s\n", option);
+      fprintf(tool_stderr, "GOT: %s\n", option);
 #endif
 
       /* pass spaces and separator(s) */
@@ -221,7 +221,7 @@ int parseconfig(const char *filename, struct GlobalConfig *global)
       }
 
 #ifdef DEBUG_CONFIG
-      fprintf(stderr, "PARAM: \"%s\"\n",(param ? param : "(null)"));
+      fprintf(tool_stderr, "PARAM: \"%s\"\n",(param ? param : "(null)"));
 #endif
       res = getparameter(option, param, NULL, &usedarg, global, operation);
       operation = global->last;

diff --git a/src/tool_progress.c b/src/tool_progress.c
index 782ad3b79017a8f252b25aaff9bd7d4db96254d0..db8679deb03dcfb6d3358189f32fb54993771337 100644 (file)
--- a/src/tool_progress.c
+++ b/src/tool_progress.c
@@ -173,7 +173,7 @@ bool progress_meter(struct GlobalConfig *global,
     header = TRUE;
     fputs("DL% UL%  Dled  Uled  Xfers  Live "
           "Total     Current  Left    Speed\n",
-          stderr);
+          tool_stderr);
   }
   if(final || (diff > 500)) {
     char time_left[10];
@@ -275,7 +275,7 @@ bool progress_meter(struct GlobalConfig *global,
     }
     time2str(time_spent, spent);
 
-    fprintf(stderr,
+    fprintf(tool_stderr,
             "\r"
             "%-3s " /* percent downloaded */
             "%-3s " /* percent uploaded */

diff --git a/src/tool_setup.h b/src/tool_setup.h
index 1a723f88ba08c2e61f49ae98c3477cce80d4e398..48b3556402dccb33b92fa800393b8012e16b6632 100644 (file)
--- a/src/tool_setup.h
+++ b/src/tool_setup.h
@@ -39,13 +39,6 @@
 
 extern FILE *tool_stderr;
 
-#if !defined(CURL_DO_NOT_OVERRIDE_STDERR) && !defined(UNITTESTS)
-#ifdef stderr
-#undef stderr
-#endif
-#define stderr tool_stderr
-#endif
-
 /*
  * curl tool certainly uses libcurl's external interface.
  */

diff --git a/src/tool_stderr.c b/src/tool_stderr.c
index 23643df7cfe885883d8a8816123c3f17dca338cf..4ab5516ae4a9cb3e7df38fa61fe7b9355debd111 100644 (file)
--- a/src/tool_stderr.c
+++ b/src/tool_stderr.c
@@ -22,20 +22,17 @@
  *
  ***************************************************************************/
 
-/* In this file, stdio.h's stderr macro is not overridden. */
-#define CURL_DO_NOT_OVERRIDE_STDERR
-
 #include "tool_setup.h"
 #include "tool_stderr.h"
 #include "tool_msgs.h"
 
 #include "memdebug.h" /* keep this as LAST include */
 
-/* In other tool files stderr is defined as tool_stderr by tool_setup.h */
 FILE *tool_stderr;
 
 void tool_init_stderr(void)
 {
+  /* !checksrc! disable STDERR 1 */
   tool_stderr = stderr;
 }
 
@@ -62,11 +59,13 @@ void tool_set_stderr_file(struct GlobalConfig *global, char *filename)
 
   /* freopen the actual stderr (stdio.h stderr) instead of tool_stderr since
      the latter may be set to stdout. */
+  /* !checksrc! disable STDERR 1 */
   fp = freopen(filename, FOPEN_WRITETEXT, stderr);
   if(!fp) {
     /* stderr may have been closed by freopen. there is nothing to be done. */
     DEBUGASSERT(0);
     return;
   }
+  /* !checksrc! disable STDERR 1 */
   tool_stderr = stderr;
 }

diff --git a/src/tool_urlglob.c b/src/tool_urlglob.c
index 4aa2e95bde8e95e4ba4db0baaf88ff7b2cae006a..69016179deb4b0fab5390c7442ab6a3fb02256e0 100644 (file)
--- a/src/tool_urlglob.c
+++ b/src/tool_urlglob.c
@@ -668,7 +668,7 @@ CURLcode glob_match_url(char **result, char *filename, struct URLGlob *glob)
           appendlen = strlen(numbuf);
           break;
         default:
-          fprintf(stderr, "internal error: invalid pattern type (%d)\n",
+          fprintf(tool_stderr, "internal error: invalid pattern type (%d)\n",
                   (int)pat->type);
           curlx_dyn_free(&dyn);
           return CURLE_FAILED_INIT;

diff --git a/src/tool_writeout.c b/src/tool_writeout.c
index 0fc955d5b4e07f9e48d0e93a633f7b6831b78c8a..981065c9ef6b90f290e468e8226b937bb304bb17 100644 (file)
--- a/src/tool_writeout.c
+++ b/src/tool_writeout.c
@@ -566,7 +566,7 @@ void ourWriteOut(struct OperationConfig *config, struct per_transfer *per,
                 if(fclose_stream)
                   fclose(stream);
                 fclose_stream = FALSE;
-                stream = stderr;
+                stream = tool_stderr;
                 break;
               case VAR_JSON:
                 ourWriteOutJSON(stream, variables, per, per_result);
@@ -583,7 +583,8 @@ void ourWriteOut(struct OperationConfig *config, struct per_transfer *per,
             }
           }
           if(!match) {
-            fprintf(stderr, "curl: unknown --write-out variable: '%.*s'\n",
+            fprintf(tool_stderr,
+                    "curl: unknown --write-out variable: '%.*s'\n",
                     (int)vlen, ptr);
           }
           ptr = end + 1; /* pass the end */