unofficial mirror of libc-alpha@sourceware.org
 help / color / mirror / Atom feed
From: "Zack Weinberg" <zack@owlfolio.org>
To: "Rich Felker" <dalias@libc.org>, "Florian Weimer" <fweimer@redhat.com>
Cc: "Gabriel Ravier" <gabravier@gmail.com>,
	"Skyler Ferrante (RIT Student)" <sjf5462@rit.edu>,
	musl@lists.openwall.com, "Andreas Schwab" <schwab@suse.de>,
	"Alejandro Colomar" <alx@kernel.org>,
	"Thorsten Glaser" <tg@mirbsd.de>, NRK <nrk@disroot.org>,
	"Guillem Jover" <guillem@hadrons.org>,
	"GNU libc development" <libc-alpha@sourceware.org>,
	libbsd@lists.freedesktop.org,
	"Serge E. Hallyn" <serge@hallyn.com>,
	"Iker Pedrosa" <ipedrosa@redhat.com>,
	"Christian Brauner" <christian@brauner.io>
Subject: Re: [musl] Re: Tweaking the program name for <err.h> functions
Date: Tue, 12 Mar 2024 15:25:31 -0400	[thread overview]
Message-ID: <180c5e67-5e97-4ac8-ab7c-696d2aa865ed@app.fastmail.com> (raw)
In-Reply-To: <20240312144225.GC4163@brightrain.aerifal.cx>



On Tue, Mar 12, 2024, at 10:42 AM, Rich Felker wrote:
> On Tue, Mar 12, 2024 at 03:31:04PM +0100, Florian Weimer wrote:
>> * Zack Weinberg:
>> 
>> > On Tue, Mar 12, 2024, at 9:54 AM, Florian Weimer wrote:
>> >>> Doing this would break many programs, such as:
>> >>> - most of coreutils, e.g. programs like ls, cat or head, since they
>> >>>   always `close` their input and output descriptors (when they've
>> >>>   written or read something) to make sure to diagnose all errors
>> >>
>> >> A slightly better way to do this is to do fflush (stdout) followed by
>> >> error checking on close (dup (fileno (stdout))).
>> >
>> > Does that actually report delayed write errors?  As you have it,
>> > the close() just drops the fd created by the dup(), the OFD is
>> > still referenced by fd 1 and therefore remains open.
>> 
>> I don't think the VFS close action is subject to reference counting.
>> Otherwise the current coreutils error checking wouldn't work because in
>> many cases, another process retains a reference to the OFD.
>
> It is. close only reports errors if it's the last fd referring to the
> ofd. It's an incredibly stupid design choice by NFS that mismatches
> expected fd behavior. 

I do fully agree that this is a design error in NFS and in close(2) more generally [like all destructors, it should be _impossible_ for close to fail], but there is no realistic prospect of changing it, and I've been burned a few times by programs that didn't notice delayed write errors. The risk of missing an error because another process still has a ref to the OFD is real, but it comes up rarely enough that I don't think it should deter us from trying to get the "only one process has this file open" case right. (Think shell `>` redirection.)

Also, I have written programs that expected fds 0 and/or 1 to be pipes, and closed them well before exiting as a cheap way to send one-shot events to the process on the other end of the pipe. I think that's a legitimate way to use inherited fds, and it would be significantly more difficult to do in some contexts if you had to use a fd > 2 to do it (e.g. the process on the other end used popen() to start things).

If there's something we can do to make it easier for programmers to do the Right Thing with closing standard fds, IMHO we should. For stdio, we perfectly well _could_ special case fclose(), when the fileno is 0/1/2, to do the dup/close dance and leave both the FILE and the fd in a valid, stubbed state. (For stdin, open("/dev/null", O_RDONLY) is clearly an appropriate stub semantic; for stdout and stderr I am unsure whether "discard all writes silently" or "fail all writes" would be better.) I don't think it's safe to make close() special case 0/1/2, though—not least because it's supposed to be atomic and async signal safe. And we should maybe give some thought to runtimes for languages other than C, which probably have their own buffering wrappers for fds 0/1/2 and might care about cross-language interop.

zw

  reply	other threads:[~2024-03-12 19:26 UTC|newest]

Thread overview: 44+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-07 22:24 Tweaking the program name for <err.h> functions Alejandro Colomar
2024-03-08  0:30 ` Guillem Jover
2024-03-08  0:47   ` enh
2024-03-08  0:52   ` Alejandro Colomar
2024-03-09 15:02     ` [musl] " Rich Felker
2024-03-09 15:49       ` Alejandro Colomar
2024-03-09 18:35         ` Andreas Schwab
2024-03-09 18:46           ` Alejandro Colomar
2024-03-09 19:18             ` [musl] " Markus Wichmann
2024-03-09 19:25             ` Rich Felker
2024-03-09 21:44         ` Thorsten Glaser
2024-03-10  6:01         ` NRK
2024-03-10 13:17           ` Alejandro Colomar
2024-03-10 14:01             ` NRK
2024-03-10 19:39               ` Rich Felker
2024-03-10 22:25                 ` Alejandro Colomar
2024-03-10 23:22                 ` Thorsten Glaser
2024-03-10 23:44                   ` Rich Felker
2024-03-11  0:19                     ` Thorsten Glaser
2024-03-11  0:46                       ` Alejandro Colomar
2024-03-11 14:46                         ` Skyler Ferrante (RIT Student)
2024-03-11 15:09                           ` Andreas Schwab
2024-03-11 15:30                             ` Skyler Ferrante (RIT Student)
2024-03-11 18:23                               ` Florian Weimer
2024-03-11 18:48                                 ` Skyler Ferrante (RIT Student)
2024-03-11 19:05                                   ` enh
2024-03-11 19:44                                     ` Rich Felker
2024-03-11 20:35                                       ` enh
2024-03-11 19:47                               ` Rich Felker
2024-03-11 20:08                                 ` Skyler Ferrante (RIT Student)
2024-03-11 20:39                                   ` enh
2024-03-11 21:21                                 ` Laurent Bercot
2024-03-11 22:05                                 ` Thorsten Glaser
2024-03-12  0:18                                 ` Gabriel Ravier
2024-03-12  0:43                                   ` Rich Felker
2024-03-12  3:23                                     ` Gabriel Ravier
2024-03-12 14:44                                       ` Rich Felker
2024-03-12 13:54                                   ` Florian Weimer
2024-03-12 14:21                                     ` Zack Weinberg
2024-03-12 14:31                                       ` Florian Weimer
2024-03-12 14:42                                         ` Rich Felker
2024-03-12 19:25                                           ` Zack Weinberg [this message]
2024-03-12 21:19                                             ` Rich Felker
2024-03-13  8:28                                             ` Florian Weimer

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://www.gnu.org/software/libc/involved.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=180c5e67-5e97-4ac8-ab7c-696d2aa865ed@app.fastmail.com \
    --to=zack@owlfolio.org \
    --cc=alx@kernel.org \
    --cc=christian@brauner.io \
    --cc=dalias@libc.org \
    --cc=fweimer@redhat.com \
    --cc=gabravier@gmail.com \
    --cc=guillem@hadrons.org \
    --cc=ipedrosa@redhat.com \
    --cc=libbsd@lists.freedesktop.org \
    --cc=libc-alpha@sourceware.org \
    --cc=musl@lists.openwall.com \
    --cc=nrk@disroot.org \
    --cc=schwab@suse.de \
    --cc=serge@hallyn.com \
    --cc=sjf5462@rit.edu \
    --cc=tg@mirbsd.de \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).