tool: improve --stderr handling

- freopen stderr with the user-specified file (--stderr file) instead of
  using a separate 'errors' stream.

- In tool_setup.h override stdio.h's stderr macro as global variable
  tool_stderr.

Both freopen and overriding the stderr macro are necessary because if
the user-specified filename is "-" then stdout is assigned to
tool_stderr and no freopen takes place. See the PR for more information.

Ref: https://github.com/curl/curl/issues/10491

Closes https://github.com/curl/curl/pull/10673

14 files changed, 151 insertions, 60 deletions

diff --git a/src/Makefile.inc b/src/Makefile.inc
index 1ee0bd454..ec822c896 100644
--- a/src/Makefile.inc
+++ b/src/Makefile.inc
@@ -82,6 +82,7 @@ CURL_CFILES = \
   tool_paramhlp.c \
   tool_parsecfg.c \
   tool_progress.c \
+  tool_stderr.c \
   tool_strdup.c \
   tool_setopt.c \
   tool_sleep.c \
@@ -126,6 +127,7 @@ CURL_HFILES = \
   tool_setopt.h \
   tool_setup.h \
   tool_sleep.h \
+  tool_stderr.h \
   tool_strdup.h \
   tool_urlglob.h \
   tool_util.h \
diff --git a/src/tool_cb_dbg.c b/src/tool_cb_dbg.c
index a01d221fa..6a51ec8fc 100644
--- a/src/tool_cb_dbg.c
+++ b/src/tool_cb_dbg.c
@@ -48,7 +48,7 @@ int tool_debug_cb(CURL *handle, curl_infotype type,
 {
   struct OperationConfig *operation = userdata;
   struct GlobalConfig *config = operation->global;
-  FILE *output = config->errors;
+  FILE *output = stderr;
   const char *text;
   struct timeval tv;
   char timebuf[20];
@@ -80,7 +80,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 = config->errors;  /* aka stderr */
+      config->trace_stream = stderr;
     else {
       config->trace_stream = fopen(config->trace_dump, FOPEN_WRITETEXT);
       config->trace_fopened = TRUE;
diff --git a/src/tool_cb_prg.c b/src/tool_cb_prg.c
index 9b856f4cc..9c8ffb2b5 100644
--- 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 = config->global->errors;
+  bar->out = stderr;
   bar->tick = 150;
   bar->barmove = 1;
 }
diff --git a/src/tool_cfgable.h b/src/tool_cfgable.h
index 6f821be24..9a15659bc 100644
--- a/src/tool_cfgable.h
+++ b/src/tool_cfgable.h
@@ -302,8 +302,6 @@ struct GlobalConfig {
   bool silent;                    /* don't show messages, --silent given */
   bool noprogress;                /* don't show progress bar */
   bool isatty;                    /* Updated internally if output is a tty */
-  FILE *errors;                   /* Error stream, defaults to stderr */
-  bool errors_fopened;            /* Whether error stream isn't stderr */
   char *trace_dump;               /* file to dump the network trace to */
   FILE *trace_stream;
   bool trace_fopened;
diff --git a/src/tool_formparse.c b/src/tool_formparse.c
index ed83899fe..e75f5e659 100644
--- a/src/tool_formparse.c
+++ b/src/tool_formparse.c
@@ -417,8 +417,7 @@ static int read_field_headers(struct OperationConfig *config,
       if(hdrlen) {
         hdrbuf[hdrlen] = '\0';
         if(slist_append(pheaders, hdrbuf)) {
-          fprintf(config->global->errors,
-                  "Out of memory for field headers!\n");
+          fprintf(stderr, "Out of memory for field headers!\n");
           return -1;
         }
         hdrlen = 0;
@@ -428,8 +427,8 @@ static int read_field_headers(struct OperationConfig *config,
     switch(c) {
     case EOF:
       if(ferror(fp)) {
-        fprintf(config->global->errors,
-                "Header file %s read error: %s\n", filename, strerror(errno));
+        fprintf(stderr, "Header file %s read error: %s\n", filename,
+                strerror(errno));
         return -1;
       }
       return 0;    /* Done. */
@@ -585,7 +584,7 @@ static int get_param_part(struct OperationConfig *config, char endchar,
         sep = *p;
         *endpos = '\0';
         if(slist_append(&headers, hdr)) {
-          fprintf(config->global->errors, "Out of memory for field header!\n");
+          fprintf(stderr, "Out of memory for field header!\n");
           curl_slist_free_all(headers);
           return -1;
         }
diff --git a/src/tool_getparam.c b/src/tool_getparam.c
index 62609693b..17a6f5567 100644
--- a/src/tool_getparam.c
+++ b/src/tool_getparam.c
@@ -42,6 +42,7 @@
 #include "tool_parsecfg.h"
 #include "tool_main.h"
 #include "dynbuf.h"
+#include "tool_stderr.h"
 
 #include "memdebug.h" /* keep this as LAST include */
 
@@ -1036,19 +1037,7 @@ ParameterError getparameter(const char *flag, /* f or -long-flag */
         break;
 
       case 'v': /* --stderr */
-        if(strcmp(nextarg, "-")) {
-          FILE *newfile = fopen(nextarg, FOPEN_WRITETEXT);
-          if(!newfile)
-            warnf(global, "Failed to open %s!\n", nextarg);
-          else {
-            if(global->errors_fopened)
-              fclose(global->errors);
-            global->errors = newfile;
-            global->errors_fopened = TRUE;
-          }
-        }
-        else
-          global->errors = stdout;
+        tool_set_stderr_file(nextarg);
         break;
       case 'w': /* --interface */
         /* interface */
@@ -2567,9 +2556,9 @@ ParameterError parse_args(struct GlobalConfig *global, int argc,
     const char *reason = param2text(result);
 
     if(orig_opt && strcmp(":", orig_opt))
-      helpf(global->errors, "option %s: %s\n", orig_opt, reason);
+      helpf(stderr, "option %s: %s\n", orig_opt, reason);
     else
-      helpf(global->errors, "%s\n", reason);
+      helpf(stderr, "%s\n", reason);
   }
 
   curlx_unicodefree(orig_opt);
diff --git a/src/tool_main.c b/src/tool_main.c
index 62c4a597e..2b7743a7e 100644
--- a/src/tool_main.c
+++ b/src/tool_main.c
@@ -53,6 +53,7 @@
 #include "tool_vms.h"
 #include "tool_main.h"
 #include "tool_libinfo.h"
+#include "tool_stderr.h"
 
 /*
  * This is low-level hard-hacking memory leak tracking and similar. Using
@@ -156,7 +157,6 @@ static CURLcode main_init(struct GlobalConfig *config)
 
   /* Initialise the global config */
   config->showerror = FALSE;          /* show errors when silent */
-  config->errors = stderr;            /* Default errors to stderr */
   config->styled_output = TRUE;       /* enable detection */
   config->parallel_max = PARALLEL_DEFAULT;
 
@@ -196,10 +196,6 @@ static void free_globalconfig(struct GlobalConfig *config)
 {
   Curl_safefree(config->trace_dump);
 
-  if(config->errors_fopened && config->errors)
-    fclose(config->errors);
-  config->errors = NULL;
-
   if(config->trace_fopened && config->trace_stream)
     fclose(config->trace_stream);
   config->trace_stream = NULL;
@@ -250,6 +246,8 @@ int main(int argc, char *argv[])
   struct GlobalConfig global;
   memset(&global, 0, sizeof(global));
 
+  tool_init_stderr();
+
 #ifdef WIN32
   /* Undocumented diagnostic option to list the full paths of all loaded
      modules. This is purposely pre-init. */
diff --git a/src/tool_msgs.c b/src/tool_msgs.c
index d9c46d6b3..4f082d5c1 100644
--- a/src/tool_msgs.c
+++ b/src/tool_msgs.c
@@ -54,7 +54,7 @@ static void voutf(struct GlobalConfig *config,
 
     ptr = print_buffer;
     while(len > 0) {
-      fputs(prefix, config->errors);
+      fputs(prefix, stderr);
 
       if(len > width) {
         size_t cut = width-1;
@@ -67,13 +67,13 @@ static void voutf(struct GlobalConfig *config,
              max text width then! */
           cut = width-1;
 
-        (void)fwrite(ptr, cut + 1, 1, config->errors);
-        fputs("\n", config->errors);
+        (void)fwrite(ptr, cut + 1, 1, stderr);
+        fputs("\n", stderr);
         ptr += cut + 1; /* skip the space too */
         len -= cut + 1;
       }
       else {
-        fputs(ptr, config->errors);
+        fputs(ptr, stderr);
         len = 0;
       }
     }
diff --git a/src/tool_operate.c b/src/tool_operate.c
index f694f088d..4e2b82114 100644
--- a/src/tool_operate.c
+++ b/src/tool_operate.c
@@ -306,7 +306,7 @@ static CURLcode pre_transfer(struct GlobalConfig *global,
     if((per->infd == -1) || fstat(per->infd, &fileinfo))
 #endif
     {
-      helpf(global->errors, "Can't open '%s'!\n", per->uploadfile);
+      helpf(stderr, "Can't open '%s'!\n", per->uploadfile);
       if(per->infd != -1) {
         close(per->infd);
         per->infd = STDIN_FILENO;
@@ -404,10 +404,10 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
     if(!config->synthetic_error && result &&
        (!global->silent || global->showerror)) {
       const char *msg = per->errorbuffer;
-      fprintf(global->errors, "curl: (%d) %s\n", result,
+      fprintf(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, global->errors);
+        fputs(CURL_CA_CERT_ERRORMSG, stderr);
     }
     else if(config->failwithbody) {
       /* if HTTP response >= 400, return error */
@@ -415,7 +415,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(global->errors,
+          fprintf(stderr,
                   "curl: (%d) The requested URL returned error: %ld\n",
                   CURLE_HTTP_RETURNED_ERROR, code);
         result = CURLE_HTTP_RETURNED_ERROR;
@@ -448,7 +448,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
       /* something went wrong in the writing process */
       result = CURLE_WRITE_ERROR;
       if(!global->silent || global->showerror)
-        fprintf(global->errors, "curl: (%d) Failed writing body\n", result);
+        fprintf(stderr, "curl: (%d) Failed writing body\n", result);
     }
   }
 
@@ -589,8 +589,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
         /* We have written data to an output file, we truncate file
          */
         if(!global->silent)
-          fprintf(global->errors, "Throwing away %"
-                  CURL_FORMAT_CURL_OFF_T " bytes\n",
+          fprintf(stderr, "Throwing away %"  CURL_FORMAT_CURL_OFF_T " bytes\n",
                   outs->bytes);
         fflush(outs->stream);
         /* truncate file at the position where we started appending */
@@ -599,8 +598,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
           /* when truncate fails, we can't just append as then we'll
              create something strange, bail out */
           if(!global->silent || global->showerror)
-            fprintf(global->errors,
-                    "curl: (23) Failed to truncate file\n");
+            fprintf(stderr, "curl: (23) Failed to truncate file\n");
           return CURLE_WRITE_ERROR;
         }
         /* now seek to the end of the file, the position where we
@@ -615,8 +613,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
 #endif
         if(rc) {
           if(!global->silent || global->showerror)
-            fprintf(global->errors,
-                    "curl: (23) Failed seeking to end of file\n");
+            fprintf(stderr, "curl: (23) Failed seeking to end of file\n");
           return CURLE_WRITE_ERROR;
         }
         outs->bytes = 0; /* clear for next round */
@@ -641,7 +638,7 @@ static CURLcode post_per_transfer(struct GlobalConfig *global,
       /* something went wrong in the writing process */
       result = CURLE_WRITE_ERROR;
       if(!global->silent || global->showerror)
-        fprintf(global->errors, "curl: (%d) Failed writing body\n", result);
+        fprintf(stderr, "curl: (%d) Failed writing body\n", result);
     }
     if(result && config->rm_partial) {
       notef(global, "Removing output file: %s\n", outs->filename);
@@ -801,7 +798,7 @@ static CURLcode single_transfer(struct GlobalConfig *global,
       /* Unless explicitly shut off */
       result = glob_url(&inglob, infiles, &state->infilenum,
                         (!global->silent || global->showerror)?
-                        global->errors:NULL);
+                        stderr:NULL);
       if(result)
         break;
       config->state.inglob = inglob;
@@ -837,7 +834,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)?
-                            global->errors:NULL);
+                            stderr:NULL);
           if(result)
             break;
           urlnum = state->urlnum;
@@ -1096,7 +1093,7 @@ static CURLcode single_transfer(struct GlobalConfig *global,
              file output call */
 
           if(config->create_dirs) {
-            result = create_dir_hierarchy(per->outfile, global->errors);
+            result = create_dir_hierarchy(per->outfile, stderr);
             /* create_dir_hierarchy shows error upon CURLE_WRITE_ERROR */
             if(result)
               break;
@@ -1240,9 +1237,6 @@ static CURLcode single_transfer(struct GlobalConfig *global,
           }
         }
 
-        if(!global->errors)
-          global->errors = stderr;
-
         if((!per->outfile || !strcmp(per->outfile, "-")) &&
            !config->use_ascii) {
           /* We get the output to stdout and we have not got the ASCII/text
@@ -1851,7 +1845,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, global->errors);
+        my_setopt(curl, CURLOPT_STDERR, stderr);
 
         /* three new ones in libcurl 7.3: */
         my_setopt_str(curl, CURLOPT_INTERFACE, config->iface);
@@ -2517,8 +2511,7 @@ static CURLcode transfer_per_config(struct GlobalConfig *global,
 
   /* Check we have a url */
   if(!config->url_list || !config->url_list->url) {
-    helpf(global->errors, "(%d) no URL specified!\n",
-          CURLE_FAILED_INIT);
+    helpf(stderr, "(%d) no URL specified!\n", CURLE_FAILED_INIT);
     return CURLE_FAILED_INIT;
   }
 
@@ -2576,7 +2569,7 @@ static CURLcode transfer_per_config(struct GlobalConfig *global,
           if(!config->capath) {
             curl_free(env);
             curl_easy_cleanup(curltls);
-            helpf(global->errors, "out of memory\n");
+            helpf(stderr, "out of memory\n");
             return CURLE_OUT_OF_MEMORY;
           }
           capath_from_env = true;
@@ -2694,7 +2687,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(global->errors, NULL);
+      helpf(stderr, NULL);
       result = CURLE_FAILED_INIT;
     }
   }
diff --git a/src/tool_progress.c b/src/tool_progress.c
index 28edafe1e..782ad3b79 100644
--- 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",
-          global->errors);
+          stderr);
   }
   if(final || (diff > 500)) {
     char time_left[10];
@@ -275,7 +275,7 @@ bool progress_meter(struct GlobalConfig *global,
     }
     time2str(time_spent, spent);
 
-    fprintf(global->errors,
+    fprintf(stderr,
             "\r"
             "%-3s " /* percent downloaded */
             "%-3s " /* percent uploaded */
diff --git a/src/tool_setup.h b/src/tool_setup.h
index 73c60747d..5d7b2305d 100644
--- a/src/tool_setup.h
+++ b/src/tool_setup.h
@@ -37,6 +37,15 @@
 
 #include "curl_setup.h" /* from the lib directory */
 
+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
new file mode 100644
index 000000000..cd752508b
--- /dev/null
+++ b/src/tool_stderr.c
@@ -0,0 +1,72 @@
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+
+/* 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 "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)
+{
+  tool_stderr = stderr;
+}
+
+void tool_set_stderr_file(char *filename)
+{
+  FILE *fp;
+
+  if(!filename)
+    return;
+
+  if(!strcmp(filename, "-")) {
+    tool_stderr = stdout;
+    return;
+  }
+
+  /* precheck that filename is accessible to lessen the chance that the
+     subsequent freopen will fail. */
+  fp = fopen(filename, FOPEN_WRITETEXT);
+  if(!fp) {
+    fprintf(tool_stderr, "Warning: Failed to open %s!\n", filename);
+    return;
+  }
+  fclose(fp);
+
+  /* freopen the actual stderr (stdio.h stderr) instead of tool_stderr since
+     the latter may be set to stdout. */
+  fp = freopen(filename, FOPEN_WRITETEXT, stderr);
+  if(!fp) {
+    /* stderr may have been closed by freopen. there is nothing to be done. */
+    DEBUGASSERT(0);
+    return;
+  }
+  tool_stderr = stderr;
+}
diff --git a/src/tool_stderr.h b/src/tool_stderr.h
new file mode 100644
index 000000000..a3080259e
--- /dev/null
+++ b/src/tool_stderr.h
@@ -0,0 +1,31 @@
+#ifndef HEADER_CURL_TOOL_STDERR_H
+#define HEADER_CURL_TOOL_STDERR_H
+/***************************************************************************
+ *                                  _   _ ____  _
+ *  Project                     ___| | | |  _ \| |
+ *                             / __| | | | |_) | |
+ *                            | (__| |_| |  _ <| |___
+ *                             \___|\___/|_| \_\_____|
+ *
+ * Copyright (C) Daniel Stenberg, <daniel@haxx.se>, et al.
+ *
+ * This software is licensed as described in the file COPYING, which
+ * you should have received as part of this distribution. The terms
+ * are also available at https://curl.se/docs/copyright.html.
+ *
+ * You may opt to use, copy, modify, merge, publish, distribute and/or sell
+ * copies of the Software, and permit persons to whom the Software is
+ * furnished to do so, under the terms of the COPYING file.
+ *
+ * This software is distributed on an "AS IS" basis, WITHOUT WARRANTY OF ANY
+ * KIND, either express or implied.
+ *
+ * SPDX-License-Identifier: curl
+ *
+ ***************************************************************************/
+#include "tool_setup.h"
+
+void tool_init_stderr(void);
+void tool_set_stderr_file(char *filename);
+
+#endif /* HEADER_CURL_TOOL_STDERR_H */
diff --git a/src/tool_writeout.c b/src/tool_writeout.c
index d93de829a..a2e3c39bb 100644
--- a/src/tool_writeout.c
+++ b/src/tool_writeout.c
@@ -427,7 +427,7 @@ void ourWriteOut(struct OperationConfig *config, struct per_transfer *per,
                 stream = stdout;
                 break;
               case VAR_STDERR:
-                stream = config->global->errors;
+                stream = stderr;
                 break;
               case VAR_JSON:
                 ourWriteOutJSON(stream, variables, per, per_result);