Re: [PATCH v3 2/2] Use O_IGNORE_CTTY where appropriate

[Sergey Bugaev]

Hello -- that is a much more positive response than I was expecting to
get! Thank you!

On Fri, Jun 9, 2023 at 9:37 PM Paul Eggert <eggert@cs.ucla.edu> wrote:
> You could change the documentation so that it now says that flags that
> imply O_IGNORE_CTTY are also meaningful. That should be fine.

Perhaps... but there's another reason I don't particularly like the
idea of doing it on that level.

_hurd_port2fd () and _hurd_intern_fd () is something that you call
once you already have an io port. O_CREAT and O_DIRECTORY and the rest
are the flags that impact how you look it up. _hurd_port2fd would have
to second-guess "this io port is said to have been opened with O_CREAT
| O_EXCL, so it can't be a ctty". It'd be better to have the caller
(open) -- that "can see" both looking the port up and interning it --
implement this bit of logic. Not that this matters for anything,
because it would still behave the same way no matter which level we
implement it at; but it seems more appropriate to me to implement it
at that level.

Samuel, what do you think?

> > I think it's a good practice to always pass O_IGNORE_CTTY where
> > it makes sense to
>
> Does your patch do that, for every 'open'-like call?

It does. I grepped glibc for open[at]64[_nocancel] and added
O_IGNORE_CTTY everywhere where it made sense, other than in
Linux-specific code (e.g. not inside sysdeps/unix/sysv/linux). I have
also found a bunch of places where O_CLOEXEC was missing, and that
lead to commit 533deafbdf189f5fbb280c28562dd43ace2f4b0f "Use O_CLOEXEC
in more places (BZ #15722)". It might be that I have missed some more
places where O_IGNORE_CTTY could be used, of course.

> > Maybe with GCC there is a chance, considering it's a GNU project?
>
> Sure. I expect even 'git' will do it if you write the patch, as they
> care about performance. Also tar, coreutils, grep, and other "open files
> a lot" apps should benefit.

Cool, so I should try writing the patches then and see what comes out of it.

> >> I suppose fopen could add a new 'T' flag, as a GNU extension, that would
> >> add O_IGNORE_CTTY to the open flags.
> >
> > What would be the compatibility implications of this? -- what if POSIX
> > later declares that 'T' must mean something else?
>
> I wouldn't worry overmuch about that. We could ask for forgiveness later.

I'm going to look into adding it into fopen then, thanks!

> Thinking bigger - why does Hurd mess with this stuff at all? Wouldn't it
> be better if O_IGNORE_CTTY was the default, and a different flag
> (O_PAY_ATTENTION_TO_CTTY, say :-) enables the ctty dance? POSIX would
> allow that behavior, as it does not require the controlling terminal to
> be required if O_NOCTTY is not set.

Sorry, I'm failing to parse/interpret that last sentence, could you
please rephrase it? Do you perhaps mean that POSIX does not require a
newly opened terminal to become your ctty even if you don't pass
O_NOCTTY?

Please note (if you haven't already) the difference between O_NOCTTY
and O_IGNORE_CTTY: O_NOCTTY is "always in effect" on the Hurd, and
defined to 0 (so I sure hope POSIX allows this). What O_IGNORE_CTTY
does is -- well, technically it just turns off one check; but if you
*don't* pass O_IGNORE_CTTY and the check succeeds and detects that the
file you're opening (or rather: the port you're interning) refers to
your ctty (that you already have set), then it does special things to
make the new fd behave like an fd to your ctty is supposed to behave
-- you e.g. get SIGINT when ^C is typed, and you get SIGTTIN / SIGTTOU
/ EBACKGROUND when trying to do I/O on the fd while in background.

So if you do pass O_IGNORE_CTTY and the file is not your ctty, you
just get a speedup. If you do pass O_IGNORE_CTTY and the file is your
ctty, you get an fd that refers to your ctty... but doesn't behave
like a ctty fd. Why you would want the latter, I have no idea (but
also it of course wasn't me who came up with O_IGNORE_CTTY, so perhaps
there is a use case).

So it is important that (re)opening /dev/tty or /dev/stdout or just
/dev/ttyp1 (by an explicit name) behaves as expected. Requiring an
explicit flag for this would be wrong, and also there'd be even less
of a chance that anyone would actually pass it in practice than there
is now with O_IGNORE_CTTY.

Sergey