Re: [PATCH 0/4] Fix closing of standard text streams for non-glibc system

[Patrick Steinhardt <ps@pks.im>]

[-- Attachment #1: Type: text/plain, Size: 1859 bytes --]

On Mon, Aug 19, 2019 at 03:36:19PM +0200, Karel Zak wrote:
> On Wed, Aug 14, 2019 at 06:45:03PM +0200, Patrick Steinhardt wrote:
> > since commit 52aa1a661 (include/closestream: avoid close more
> > than once, 2019-06-13), util-linux fails to build on musl libc
> > based systems. The culprit here is that it introduced assignments
> > to stderr and stdout, while the C89 standard explicitly notes
> > that treating stderr and stdout as valid lvalues is not a
> > requirement for any conforming C implementation. musl libc
> > implemented these streams as `extern FILE *const`, and as a
> > result assigning to these variables causes compiler errors.
> 
> The question is if close() for stdout and stderr is the right way to
> go. 
> 
> In an ideal world it would be enough to use ferror()+fflush(),
> unfortunately for example NFS has never reached an ideal world and it
> requires fclose()... See
> 
>  https://lists.gnu.org/r/bug-gnulib/2019-04/msg00061.html
> 
> Florian (added to CC), also suggested to use dup3() for the
> descriptors and then fclose() for the new handle. It sounds like a
> pretty elegant solution how to avoid all the issues with NULL and it's
> also robust enough if you accidentally call close_stream() more than
> once.
> 
> See
> 
>  https://bugzilla.redhat.com/show_bug.cgi?id=1732450#c4
> 
> Maybe we can improve include/closestream.h to use dup3(), than it would
> be possible keep all in the header file as inline functions. 
> 
> Comments?

I honestly don't get why we'd need to use dup3(2), though.
Couldn't the same be achieved by using dup2(3P) followed by
close(3P), instead? Especially considering that the former one
isn't specified by POSIX, either, but the latter one is.

That being said, if we agree on a proper fix here then I'd be
happy to post a v2.

Regards
Patrick

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 833 bytes --]