On Jun 27, 2007, at 09:02, Jim Meyering wrote:
- if (!use_stdout)
- realstdout = fdopen(dup(1), "w");
+ if (!use_stdout) {
+ int fd = dup(1);
+ if (fd < 0 || (realstdout = fdopen(fd, "w")) == NULL)
+ die("failed to duplicate standard output: %s",
+ strerror(errno));
+ }
This makes the code unreadable! A great way to ruin
perfectly fine code is to add tons of error checking.
The error checking is likely wrong (detects non-errors,
or fails to detect real ones), and for sure makes code
untestable and unreadable.
If we really case about catching such errors, write
the code as:
if (!use_stdout)
realstdout = xfdopen(dup(1), "w");
where xfdopen is a wrapper around fdopen that dies in
case of an error. This follows a practice we use elsewhere,
and only adds one character to the code and only affects
readability very slightly.
Without this, if you ever run out of file descriptors, dup will
fail (silently), fdopen will return NULL, and fprintf will
try to dereference NULL (i.e., usually segfault).
As it is unlikely the failure mode will ever occur in practice,
any way of aborting is fine. Even SIGSEGV would do: it would be
trivial to find that we were leaking file descriptors or are out
of memory. Oh, wait, that means we don't need any checking code
at all...
-Geert
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
