Some old C code I wrote implemented an output file option, which I reduced to a minimal example:
#define _XOPEN_SOURCE
#include <stdio.h>
#include <string.h>
static void output(const char *output_path) {
FILE *output_file = stdout;
if (output_path == NULL || *output_path == '\0' || strcmp(output_path, "-") == 0) {
output_path = "stdout";
}
else {
output_file = fopen(output_path, "w");
if (output_file == NULL)
return;
}
fprintf(stderr, "Output path: %s\n", output_path);
fputs("Hello there\n", output_file);
if (output_file != stdout)
fclose(output_file);
}
int main(int argc, char *argv[]) {
/* ... */
output(argc > 1 ? argv[1] : NULL);
/* ... */
puts("Done");
return 0;
}
That is, a file is only opened and closed when a (non--
) path is provided.
Otherwise, stdout
is used and not closed since it is used for other output
later. Even if stdout
wasn't used again, I still wouldn't close it on the
advice of POSIX:
Since after the call to fclose() any use of stream results in undefined behavior, fclose() should not be used on stdin, stdout, or stderr except immediately before process termination (see XBD Process Termination), so as to avoid triggering undefined behavior in other standard interfaces that rely on these streams.
However, I recently started using gcc's static analyzer and it doesn't like this code:
$ gcc --version
gcc (Debian 13.3.0-1) 13.3.0
Copyright (C) 2023 Free Software Foundation, Inc.
This is free software; see the source for copying conditions. There is NO
warranty; not even for MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
$ gcc -fanalyzer main.c
main.c: In function ‘output’:
main.c:20:12: warning: leak of FILE ‘output_file’ [CWE-775] [-Wanalyzer-file-leak]
20 | if (output_file != stdout)
| ^
‘output’: events 1-7
|
| 8 | if (output_path == NULL || *output_path == '\0' || strcmp(output_path, "-") == 0) {
| | ^
| | |
| | (1) following ‘false’ branch...
|......
| 12 | output_file = fopen(output_path, "w");
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) ...to here
| | (3) opened here
| 13 | if (output_file == NULL)
| | ~
| | |
| | (4) assuming ‘output_file’ is non-NULL
| | (5) following ‘false’ branch (when ‘output_file’ is non-NULL)...
|......
| 17 | fprintf(stderr, "Output path: %s\n", output_path);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) ...to here
|......
| 20 | if (output_file != stdout)
| | ~
| | |
| | (7) following ‘false’ branch...
|
‘output’: event 8
|
|cc1:
| (8): ...to here
|
‘output’: event 9
|
| 20 | if (output_file != stdout)
| | ^
| | |
| | (9) ‘output_file’ leaks here; was opened at (3)
|
main.c:20:12: warning: leak of ‘output_file’ [CWE-401] [-Wanalyzer-malloc-leak]
‘output’: events 1-7
|
| 8 | if (output_path == NULL || *output_path == '\0' || strcmp(output_path, "-") == 0) {
| | ^
| | |
| | (1) following ‘false’ branch...
|......
| 12 | output_file = fopen(output_path, "w");
| | ~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (2) ...to here
| | (3) allocated here
| 13 | if (output_file == NULL)
| | ~
| | |
| | (4) assuming ‘output_file’ is non-NULL
| | (5) following ‘false’ branch (when ‘output_file’ is non-NULL)...
|......
| 17 | fprintf(stderr, "Output path: %s\n", output_path);
| | ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
| | |
| | (6) ...to here
|......
| 20 | if (output_file != stdout)
| | ~
| | |
| | (7) following ‘false’ branch...
|
‘output’: event 8
|
|cc1:
| (8): ...to here
|
‘output’: event 9
|
| 20 | if (output_file != stdout)
| | ^
| | |
| | (9) ‘output_file’ leaks here; was allocated at (3)
|
Since the analyzer is checking fopen
/fclose
I assume it understands the
behavior of these library functions completely. Its output seems to imply that
fopen
can return stdout
, and that if this happens then fclose
should be
called on it. Is this the case or am I misunderstanding the output?
dup2
andfdopen
so you'll get aFILE
you can close without issues.fopen()
will never returnstdout
(orstdin
orstderr
) as the file stream it just created". I'm not sure that's documented, but there are essentially no circumstances under which it is vaguely plausible that it could happen. If you've not closed the standard streams, they won't be returned as a new stream. […continued…]stdout
. I'm surefopen()
would not callopen()
, search through a table of open file streams, find one with the same file descriptor as was returned, and recycle the stream. That's implausible. You might end up with two files streams (stdout
and the new one) both using the same file descriptor, which would be unsatisfactory, but that's a consequence of you abusing the system by closingSTDOUT_FILENO
. While you leave the standard I/O streams and descriptors open,fopen()
won't return any standard stream.