tool: use our own stderr variable

[vszakats]

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 06133d3
Follow-up to 2f17a9b

Closes #11958

---

TOTEST: See how this work when running tests. There is no UNITTEST mode now, tool_stderr needs to be initialized to stderr for unittests. [PASSED]

[Romain-Geissler-1A]

Hi,

FYI, we are trying to build curl 8.4.0 (following the CVE announcement), and somehow the changes here makes the build (we use autotools by default, not cmake) error by default. It errors actually with what checksrc considers only as warnings (so maybe we should use some option to not stop the build ?):

09:20:40  ./tool_stderr.c:62:1: warning: STDERR already disabled from line 35 (BADCOMMAND)
09:20:40     /* !checksrc! disable STDERR 1 */
09:20:40   ^
09:20:40  ./tool_stderr.c:69:1: warning: STDERR already disabled from line 35 (BADCOMMAND)
09:20:40     /* !checksrc! disable STDERR 1 */
09:20:40   ^
09:20:40  ./tool_stderr.c:35:18: warning: Unused ignore: STDERR (UNUSEDIGNORE)
09:20:40     /* !checksrc! disable STDERR 1 */
09:20:40                    ^
09:20:40  checksrc: 0 errors and 3 warnings
09:20:40  checksrc: 0 errors and 10 warnings suppressed

and I tried to apply the following patch:

--- src/tool_stderr.c
+++ src/tool_stderr.c
@@ -32,7 +32,6 @@
 
 void tool_init_stderr(void)
 {
-  /* !checksrc! disable STDERR 1 */
   tool_stderr = stderr;
 }
 
@@ -59,13 +58,11 @@

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

which "unblocked" the build, however it really doesn't sound expected at all considering what checksrc is meant to check ! ;) So I am not sure if we are the only ones or if everyone has this problem.

[Romain-Geissler-1A]

Ah actually it's the file src/.checksrc that this pull request adds which is missing in the curl 8.4.0 tarball. It works if I apply this patch instead:

This file is committed in git, but missing from the curl 8.4.0 tarball, and makes the build fail if missing.

--- /dev/null
+++ src/.checksrc
@@ -0,0 +1 @@
+enable STDERR

I will open an issue for that, so a new release tarball is created (as everyone will try to rebuild curl following the CVE announcement).

[Romain-Geissler-1A]

@vszakats I see you have already a commit to fix the tarball generation. FYI, I reported this issue as #12084.

[vszakats]

Yes, thanks for reporting this. I've linked #12084 after noticing it.

[vszakats]

I wonder why this slipped through the distcheck CI run. Maybe
hidden files are exempt from it?