[v3,1/2] include/fcntl.h: Define O_IGNORE_CTTY

  This internal definition makes it possible to use O_IGNORE_CTTY in
the glibc codebase unconditionally, no matter whether the current port
provides it or not (i.e. both on Hurd and on Linux). Along with the
definition, this adds a small guide on when O_IGNORE_CTTY is to be used.

The following commit will actually make use of O_IGNORE_CTTY
throughout the glibc codebase.

Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
---
 include/fcntl.h | 15 +++++++++++++++
 1 file changed, 15 insertions(+)
  

On 04/06/23 17:42, Sergey Bugaev via Libc-alpha wrote:
> This internal definition makes it possible to use O_IGNORE_CTTY in
> the glibc codebase unconditionally, no matter whether the current port
> provides it or not (i.e. both on Hurd and on Linux). Along with the
> definition, this adds a small guide on when O_IGNORE_CTTY is to be used.
> 
> The following commit will actually make use of O_IGNORE_CTTY
> throughout the glibc codebase.
> 
> Signed-off-by: Sergey Bugaev <bugaevc@gmail.com>
> ---
>  include/fcntl.h | 15 +++++++++++++++
>  1 file changed, 15 insertions(+)
> 
> diff --git a/include/fcntl.h b/include/fcntl.h
> index be435047..d788db2e 100644
> --- a/include/fcntl.h
> +++ b/include/fcntl.h
> @@ -33,6 +33,21 @@ extern int __openat_2 (int __fd, const char *__path, int __oflag);
>  extern int __openat64_2 (int __fd, const char *__path, int __oflag);
>  
>  
> +/* Makes open () & friends faster on the Hurd, but can only be used (without
> +   altering user-visible behavior) when we're sure that the file we're opening
> +   is not (at the moment) our controlling terminal.  Use this when:
> +   - opening well-known files internally (utmp, nss db);
> +   - opening files with user-specified names that can not reasonably be ttys
> +     (sem_open, shm_open);
> +   - opening new (previously unused) ttys (openpty).
> +   Don't use this when:
> +   - doing a general-purpose open () with a user-controlled path that could
> +     well be "/dev/tty" (fopen).  */
> +#ifndef O_IGNORE_CTTY
> +#  define O_IGNORE_CTTY	0
> +#endif
> +

I think it would be better to add a sysdeps/unix/sysv/linux/fcntl.h which
defines O_IGNORE_CTTY  unconditionally and include the default one (either
directly or though include_next.h).  We currently are trying to avoid the 
"#ifdef ...", so a code that does not define, where is should, would fail 
at compile time.

> +
>  #if IS_IN (rtld)
>  #  include <dl-fcntl.h>
>  #endif
  

Hello,

On Mon, Jun 5, 2023 at 9:25 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
> We currently are trying to avoid the
> "#ifdef ...", so a code that does not define, where is should, would fail
> at compile time.

Yes, this makes perfect sense, and it was something I was also
slightly concerned about (what if the Hurd's real definition stops
being brought in by include/fcntl.h for some reason? -- then we'd just
silently get a 0, and nobody would notice). On the other hand I wanted
to not cause any additional troubles for other potential ports
(FreeBSD), but maybe it's fine to require them to just add their own
little header.

Do you think the Linux port should define O_IGNORE_CTTY to O_NOCTTY
and not to 0?

> I think it would be better to add a sysdeps/unix/sysv/linux/fcntl.h which
> defines O_IGNORE_CTTY  unconditionally and include the default one (either
> directly or though include_next.h).

Could you please clarify how this whole system of file overrides
works? (I mean: a more specific sysdep file, for some unclear
definition of "specific", automatically overriding a less specific
file of the same basename.)

I think I've seen vpath get used somewhere, so I would guess that the
sysdep (and other) directories are added to vpath order by their
priority, and whichever one Make finds first, it passes to the
compiler. Header files, I would guess again, are simply handled by
passing all the paths (once again properly sorted) with -I, and it's
the compiler that looks for the first directory that contains file of
the given name -- this makes it possible to #include_next, and this
must also be how include/ contains headers that are used during libc
compilation but not installed (include/ must not be on the vpath
then?).

But this brings me to the more specific question: the headers to be
installed are also found using vpath during 'make install', right? How
would this work, will Make somehow know to not install this
sysdeps/unix/sysv/linux/fcntl.h file that you're proposing, and keep
installing io/fcntl.h?

Sergey
  

On 09/06/23 06:29, Sergey Bugaev wrote:
> Hello,
> 
> On Mon, Jun 5, 2023 at 9:25 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>> We currently are trying to avoid the
>> "#ifdef ...", so a code that does not define, where is should, would fail
>> at compile time.
> 
> Yes, this makes perfect sense, and it was something I was also
> slightly concerned about (what if the Hurd's real definition stops
> being brought in by include/fcntl.h for some reason? -- then we'd just
> silently get a 0, and nobody would notice). On the other hand I wanted
> to not cause any additional troubles for other potential ports
> (FreeBSD), but maybe it's fine to require them to just add their own
> little header.

Yeah that's the idea, but by adding a generic one it would be required
iff the kernel/system needs to something differente.

> 
> Do you think the Linux port should define O_IGNORE_CTTY to O_NOCTTY
> and not to 0?

Hurd O_IGNORE_CTTY and Linux O_NOCTTY do not have the *exactly* semantic,
so I think we should avoid change the internal open flags in this
specific patch.

> 
>> I think it would be better to add a sysdeps/unix/sysv/linux/fcntl.h which
>> defines O_IGNORE_CTTY  unconditionally and include the default one (either
>> directly or though include_next.h).
> 
> Could you please clarify how this whole system of file overrides
> works? (I mean: a more specific sysdep file, for some unclear
> definition of "specific", automatically overriding a less specific
> file of the same basename.)

We have the internal header file, to say 'include/fcntl.h', which is
used when building glibc itself including the tests.  The internal-only
header also includes the installed one, 'io/fcntl.h', which defines
the ABI glibc provides.

So to override a internal-only definition we have some options:

  1. Override the file which is has precedence in the sysdeps selection
     (which defines the -I at built time).  So if you add the file
     "sysdeps/unix/sysv/linux/include/fcntl.h'., it would be included 
     instead of 'include/fcntl.h'.
     To avoid the need to replicate the same prototypes and definitions
     on the generic 'include/fcntl.h' in the system specific one we
     can use the include_next extension (check on how the sysvipc code
     done, like include/sys/sem.h).

  2. Add per-system file that is included in the generic 'include/fcntl.h',
     for instance fcntl-system.h or something like that.  On then add
     a generic definition on sysdep/generic/ with the expected value
     and override it on any sysdep folder that requires it.

I tend to see the second options as a slight simpler one.

> 
> I think I've seen vpath get used somewhere, so I would guess that the
> sysdep (and other) directories are added to vpath order by their
> priority, and whichever one Make finds first, it passes to the
> compiler. Header files, I would guess again, are simply handled by
> passing all the paths (once again properly sorted) with -I, and it's
> the compiler that looks for the first directory that contains file of
> the given name -- this makes it possible to #include_next, and this
> must also be how include/ contains headers that are used during libc
> compilation but not installed (include/ must not be on the vpath
> then?).
> 
> But this brings me to the more specific question: the headers to be
> installed are also found using vpath during 'make install', right? How
> would this work, will Make somehow know to not install this
> sysdeps/unix/sysv/linux/fcntl.h file that you're proposing, and keep
> installing io/fcntl.h?

Afaiu there is no need to install any new header for this, this is an
internal only definition to use on open call within glibc.
  

Hello,

On Mon, Jun 12, 2023 at 9:56 PM Adhemerval Zanella Netto
<adhemerval.zanella@linaro.org> wrote:
> > Do you think the Linux port should define O_IGNORE_CTTY to O_NOCTTY
> > and not to 0?
>
> Hurd O_IGNORE_CTTY and Linux O_NOCTTY do not have the *exactly* semantic,
> so I think we should avoid change the internal open flags in this
> specific patch.

OK.

> We have the internal header file, to say 'include/fcntl.h', which is
> used when building glibc itself including the tests.  The internal-only
> header also includes the installed one, 'io/fcntl.h', which defines
> the ABI glibc provides.

Yes, that I understand.

> So to override a internal-only definition we have some options:
>
>   1. Override the file which is has precedence in the sysdeps selection
>      (which defines the -I at built time).  So if you add the file
>      "sysdeps/unix/sysv/linux/include/fcntl.h'., it would be included
>      instead of 'include/fcntl.h'.

Oh cool, I didn't realize it was possible to nest include/ under
sysdeps/, thank you! I thought there was only the top-level include/
directory, but now I see that there are more, including even a few
Mach/Hurd specific ones.

>   2. Add per-system file that is included in the generic 'include/fcntl.h',
>      for instance fcntl-system.h or something like that.  On then add
>      a generic definition on sysdep/generic/ with the expected value
>      and override it on any sysdep folder that requires it.

Is there already an example of such a header file? I can't find any
files in any include/ directory matching either *sysdep* or *system*.

> I tend to see the second options as a slight simpler one.

The first option sounds simpler to me.

Should it be sysdeps/unix/sysv/linux/include/fcntl.h or
sysdeps/generic/include/fcntl.h? If it's the latter, I'd have to
create a sysdeps/hurd/include/fcntl.h that does not (re)define
O_IGNORE_CTTY (right?).

> > But this brings me to the more specific question: the headers to be
> > installed are also found using vpath during 'make install', right? How
> > would this work, will Make somehow know to not install this
> > sysdeps/unix/sysv/linux/fcntl.h file that you're proposing, and keep
> > installing io/fcntl.h?
>
> Afaiu there is no need to install any new header for this, this is an
> internal only definition to use on open call within glibc.

Yes, this is my point: you were suggesting that I add
sysdeps/unix/sysv/linux/fcntl.h (notice no /include/ in the middle),
which -- I'm not sure whether it would get installed by the build
system (instead of io/fcntl.h) or not. I want it to *not* get
installed. The option with /include/ in the middle that you're
suggesting now should take care of that.

Sergey
  

On 13/06/23 06:42, Sergey Bugaev wrote:
> Hello,
> 
> On Mon, Jun 12, 2023 at 9:56 PM Adhemerval Zanella Netto
> <adhemerval.zanella@linaro.org> wrote:
>>> Do you think the Linux port should define O_IGNORE_CTTY to O_NOCTTY
>>> and not to 0?
>>
>> Hurd O_IGNORE_CTTY and Linux O_NOCTTY do not have the *exactly* semantic,
>> so I think we should avoid change the internal open flags in this
>> specific patch.
> 
> OK.
> 
>> We have the internal header file, to say 'include/fcntl.h', which is
>> used when building glibc itself including the tests.  The internal-only
>> header also includes the installed one, 'io/fcntl.h', which defines
>> the ABI glibc provides.
> 
> Yes, that I understand.
> 
>> So to override a internal-only definition we have some options:
>>
>>   1. Override the file which is has precedence in the sysdeps selection
>>      (which defines the -I at built time).  So if you add the file
>>      "sysdeps/unix/sysv/linux/include/fcntl.h'., it would be included
>>      instead of 'include/fcntl.h'.
> 
> Oh cool, I didn't realize it was possible to nest include/ under
> sysdeps/, thank you! I thought there was only the top-level include/
> directory, but now I see that there are more, including even a few
> Mach/Hurd specific ones.
This comes from how the sysdeps (or sysnames from configuration) is organized
and used as include directories, it allows system specific path (which as
more 'deep' in sysdeps) to take precedence over default ones.

> 
>>   2. Add per-system file that is included in the generic 'include/fcntl.h',
>>      for instance fcntl-system.h or something like that.  On then add
>>      a generic definition on sysdep/generic/ with the expected value
>>      and override it on any sysdep folder that requires it.
> 
> Is there already an example of such a header file? I can't find any
> files in any include/ directory matching either *sysdep* or *system*.

Usually for system specific headers we do not add them on include/ but
rather on sysdeps/generic and override when necessary.  For instance
the sysdeps/generic/math_ldbl.h which is override multiple times
because of the multiple long double ABIs we support

Another example is the malloc-hugepages.c, which has a common definition
but the implementation is reimplemented for Linux.

> 
>> I tend to see the second options as a slight simpler one.
> 
> The first option sounds simpler to me.
> 
> Should it be sysdeps/unix/sysv/linux/include/fcntl.h or
> sysdeps/generic/include/fcntl.h? If it's the latter, I'd have to
> create a sysdeps/hurd/include/fcntl.h that does not (re)define
> O_IGNORE_CTTY (right?).

I think for this it would be better to just add a sysdeps/generic/fcntl-system.h
(or a better name if you a better idea), and define O_IGNORE_CTTY as 0.
The Hurd version will then have a different value.  Then include it on 
include/fcntl.h.

Btw, we already do it for dl-fcntl.h to handle another Hurd requirement.

> 
>>> But this brings me to the more specific question: the headers to be
>>> installed are also found using vpath during 'make install', right? How
>>> would this work, will Make somehow know to not install this
>>> sysdeps/unix/sysv/linux/fcntl.h file that you're proposing, and keep
>>> installing io/fcntl.h?
>>
>> Afaiu there is no need to install any new header for this, this is an
>> internal only definition to use on open call within glibc.
> 
> Yes, this is my point: you were suggesting that I add
> sysdeps/unix/sysv/linux/fcntl.h (notice no /include/ in the middle),
> which -- I'm not sure whether it would get installed by the build
> system (instead of io/fcntl.h) or not. I want it to *not* get
> installed. The option with /include/ in the middle that you're
> suggesting now should take care of that.

Afaik none of the header on include are installed, each subdir Makefile
needs to explicit add the expected file on 'headers' rule.  Also, for
sysdeps we have an extra rule, sysdep_headers, that adds extra includes.