You're correct. All callers invoke pg_fatal() on failure, so the
process exits immediately and the OS reclaims the fd. There is no
live bug worth back-patching on those grounds.
That said, the patch does fix a real diagnostic problem. In the
original code, when dup() fails with EMFILE, the -1 return value is
passed directly to fdopen(), which fails with EBADF. The user sees:
pg_dump: error: could not open output file: Bad file descriptor
which is misleading -- the actual cause is fd exhaustion, not a bad
descriptor. With the patch, errno is preserved correctly, so the
message becomes:
pg_dump: error: could not open output file: Too many open files
which gives the user actionable information.
I'm happy to limit this to HEAD only if back-patching is not
warranted.
Regards,
Jianghua Yang
Jianghua Yang <yjhjstz@gmail.com> writes:
> == The Bug ==
> All four compression open functions use this pattern when an existing
> file descriptor is passed in:
> if (fd >= 0)
> fp = fdopen(dup(fd), mode); /* or gzdopen() */
> if (fp == NULL)
> return false; /* dup'd fd is leaked here */
> The problem is that dup(fd) and fdopen()/gzdopen() are two separate
> steps, and their failure modes must be handled independently:
Hmm. You're right that we could leak the dup'd FD, but would it matter?
I'm pretty sure all these programs will just exit immediately on
failure.
I'm not averse to improving the code, but I'm not sure there is
a live bug worth back-patching.
regards, tom lane
