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

[Adhemerval Zanella Netto]

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.