Header conformance/improvements (part 2)

Header conformance/improvements (part 2)

[Daniel Sabogal]

Here's a list of observations from musl's headers.

tar.h
-----
* TSVTX
this constant is XSI-shaded
glibc exposes it with _XOPEN_SOURCE

unistd.h
--------
* F_LOCK, F_TEST, F_TLOCK, F_ULOCK
these constants are XSI-shaded
glibc exposes them with _XOPEN_SOURCE

stropts.h
---------
* RPROTMASK
this constant is non-standard and not reserved
glibc exposes it with _GNU_SOURCE

signal.h
--------
* int sigqueue(pid_t, int, /* const */ union sigval);
harmless; it just doesn't reflect http://austingroupbugs.net/view.php?id=844

arch/*/bits/termios.h
---------------------
* NLDLY, NL0, NL1
* CRDLY, CR0, CR1, CR2, CR3
* TABDLY, TAB0, TAB1, TAB2, TAB3
* BSDLY, BS0, BS1, FFDLY, FF0, FF1
these constants are XSI-shaded
(so are VTDLY, VT0 and VT1, but the prefix "V" is reserved by posix)
glibc exposes them with _XOPEN_SOURCE

limits.h
--------
* PAGE_SIZE
* NL_LANGMAX
* NZERO
these constants are XSI-shaded
glibc exposes them with _XOPEN_SOURCE (except PAGE_SIZE)

sys/socket.h
------------
* AF_* excluding AF_{INET,INET6,UNIX,UNSPEC}
* MSG_* excluding MSG_{CTRUNC,DONTROUTE,EOR,OOB,NOSIGNAL,PEEK,TRUNC,WAITALL}
* PF_*
* SCM_* excluding SCM_RIGHTS
* SO* excluding SOCK_{DGRAM,RAW,SEQPACKET,STREAM},
SO_{ACCEPTCONN,BROADCAST,DEBUG,DONTROUTE,ERROR,KEEPALIVE,LINGER,OOBINLINE,RCVBUF,RCVLOWAT,RCVTIMEO,REUSEADDR,SNDBUF,SNDLOWAT,SNDTIMEO,TYPE},
SOL_SOCKET, and SOMAXCONN
* CMSG_* excluding CMSG_{DATA,NXTHDR,FIRSTHDR}
these constants/macros are reserved by an XSI-shaded prefix
changing this might be too intrusive; glibc just exposes them

inttypes.h
----------
* wchar_t
this symbol is exposed to the ISO C namespace
AFAICT, this symbol is CX-shaded, and according to n1570 7.8.2.4p1,
it seems to be intended that <stddef.h> be included to expose wchar_t

wchar.h
-------
* FILE
this symbol is exposed to the ISO C namespace
AFAICT, this symbol is CX-shaded, and according to n1570 7.29.2.1p1,
it seems to be intended that <stdio.h> be included to expose FILE

Re: Header conformance/improvements (part 2)

[Rich Felker]

On Fri, Feb 23, 2018 at 01:32:49PM -0500, Daniel Sabogal wrote:
> Here's a list of observations from musl's headers.

Thanks!

> tar.h
> -----
> * TSVTX
> this constant is XSI-shaded
> glibc exposes it with _XOPEN_SOURCE

tar.h is not governed by any modern standard. Not sure if there's any
reason to change it. I feel like making it depend on FTMs is wrong.

> unistd.h
> --------
> * F_LOCK, F_TEST, F_TLOCK, F_ULOCK
> these constants are XSI-shaded
> glibc exposes them with _XOPEN_SOURCE

Indeed they go with lockf and should be in the #ifdef with it.

> stropts.h
> ---------
> * RPROTMASK
> this constant is non-standard and not reserved
> glibc exposes it with _GNU_SOURCE

Aside from the ioctl function, this is all deprecated/removed
functionality, not governed by any profile of the standards we claim
to support. Not sure if there's any reason to change it.

> signal.h
> --------
> * int sigqueue(pid_t, int, /* const */ union sigval);
> harmless; it just doesn't reflect http://austingroupbugs.net/view.php?id=844

I don't think this is any actual diference; the const keyword is a nop
there. Issue 844 is just about the standard gratuitously including a
do-nothing keyword there.

> arch/*/bits/termios.h
> ---------------------
> * NLDLY, NL0, NL1
> * CRDLY, CR0, CR1, CR2, CR3
> * TABDLY, TAB0, TAB1, TAB2, TAB3
> * BSDLY, BS0, BS1, FFDLY, FF0, FF1
> these constants are XSI-shaded
> (so are VTDLY, VT0 and VT1, but the prefix "V" is reserved by posix)
> glibc exposes them with _XOPEN_SOURCE

This probably should be fixed; unfortunately it means moving some FTM
logic into bits headers or refactoring.

> limits.h
> --------
> * PAGE_SIZE
> * NL_LANGMAX
> * NZERO
> these constants are XSI-shaded
> glibc exposes them with _XOPEN_SOURCE (except PAGE_SIZE)

OK, for PAGE_SIZE, the arch bits should be changed to define PAGESIZE
instead of PAGE_SIZE and the top-level limits.h logic should be
reversed to define PAGE_SIZE in terms of PAGESIZE. The others are
defined in top-level file anyway and just need to be moved under
proper FTMs.

> sys/socket.h
> ------------
> * AF_* excluding AF_{INET,INET6,UNIX,UNSPEC}
> * MSG_* excluding MSG_{CTRUNC,DONTROUTE,EOR,OOB,NOSIGNAL,PEEK,TRUNC,WAITALL}
> * PF_*
> * SCM_* excluding SCM_RIGHTS
> * SO* excluding SOCK_{DGRAM,RAW,SEQPACKET,STREAM},
> SO_{ACCEPTCONN,BROADCAST,DEBUG,DONTROUTE,ERROR,KEEPALIVE,LINGER,OOBINLINE,RCVBUF,RCVLOWAT,RCVTIMEO,REUSEADDR,SNDBUF,SNDLOWAT,SNDTIMEO,TYPE},
> SOL_SOCKET, and SOMAXCONN
> * CMSG_* excluding CMSG_{DATA,NXTHDR,FIRSTHDR}
> these constants/macros are reserved by an XSI-shaded prefix
> changing this might be too intrusive; glibc just exposes them

This is surprising. I doubt it would hurt to change, since little
stuff builds with base POSIX profile anyway, but I'm not in a hurry to
make changes here if not needed.

> inttypes.h
> ----------
> * wchar_t
> this symbol is exposed to the ISO C namespace
> AFAICT, this symbol is CX-shaded, and according to n1570 7.8.2.4p1,
> it seems to be intended that <stddef.h> be included to expose wchar_t

Ah. This is problematic because functions declared in inttypes.h
require wchar_t to prototype. Of course a shadow name for the same
type can be defined (like how va_list is handled) but it's ugly...

> wchar.h
> -------
> * FILE
> this symbol is exposed to the ISO C namespace
> AFAICT, this symbol is CX-shaded, and according to n1570 7.29.2.1p1,
> it seems to be intended that <stdio.h> be included to expose FILE

Similar issue. It could be fixed with a shadot typedef or explicit
"struct _IO_FILE". The latter is ugly and something of a violation of
the abstraction, I think..

Rich

Re: Header conformance/improvements (part 2)

[Daniel Sabogal]

On Fri, Feb 23, 2018 at 2:20 PM, Rich Felker <dalias@libc.org> wrote:
> On Fri, Feb 23, 2018 at 01:32:49PM -0500, Daniel Sabogal wrote:
>
>> tar.h
>> -----
>> * TSVTX
>> this constant is XSI-shaded
>> glibc exposes it with _XOPEN_SOURCE
>
> tar.h is not governed by any modern standard. Not sure if there's any
> reason to change it. I feel like making it depend on FTMs is wrong.

I see that tar.h is specified here:
http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/tar.h.html#tag_13_73

>> stropts.h
>> ---------
>> * RPROTMASK
>> this constant is non-standard and not reserved
>> glibc exposes it with _GNU_SOURCE
>
> Aside from the ioctl function, this is all deprecated/removed
> functionality, not governed by any profile of the standards we claim
> to support. Not sure if there's any reason to change it.

This header is obsolescent so it probably doesn't matter.

>> signal.h
>> --------
>> * int sigqueue(pid_t, int, /* const */ union sigval);
>> harmless; it just doesn't reflect http://austingroupbugs.net/view.php?id=844
>
> I don't think this is any actual diference; the const keyword is a nop
> there. Issue 844 is just about the standard gratuitously including a
> do-nothing keyword there.

Right. Keeping the qualifier here is harmless.

>> inttypes.h
>> ----------
>> * wchar_t
>> this symbol is exposed to the ISO C namespace
>> AFAICT, this symbol is CX-shaded, and according to n1570 7.8.2.4p1,
>> it seems to be intended that <stddef.h> be included to expose wchar_t
>
> Ah. This is problematic because functions declared in inttypes.h
> require wchar_t to prototype. Of course a shadow name for the same
> type can be defined (like how va_list is handled) but it's ugly...
>
>> wchar.h
>> -------
>> * FILE
>> this symbol is exposed to the ISO C namespace
>> AFAICT, this symbol is CX-shaded, and according to n1570 7.29.2.1p1,
>> it seems to be intended that <stdio.h> be included to expose FILE
>
> Similar issue. It could be fixed with a shadot typedef or explicit
> "struct _IO_FILE". The latter is ugly and something of a violation of
> the abstraction, I think..

Indeed. I think ISO C should have exposed these data types.

Re: Header conformance/improvements (part 2)

[Rich Felker]

On Fri, Feb 23, 2018 at 02:48:10PM -0500, Daniel Sabogal wrote:
> On Fri, Feb 23, 2018 at 2:20 PM, Rich Felker <dalias@libc.org> wrote:
> > On Fri, Feb 23, 2018 at 01:32:49PM -0500, Daniel Sabogal wrote:
> >
> >> tar.h
> >> -----
> >> * TSVTX
> >> this constant is XSI-shaded
> >> glibc exposes it with _XOPEN_SOURCE
> >
> > tar.h is not governed by any modern standard. Not sure if there's any
> > reason to change it. I feel like making it depend on FTMs is wrong.
> 
> I see that tar.h is specified here:
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/tar.h.html#tag_13_73

Uhg, how could I forget? OK, I'll fix this one.

> >> signal.h
> >> --------
> >> * int sigqueue(pid_t, int, /* const */ union sigval);
> >> harmless; it just doesn't reflect http://austingroupbugs.net/view.php?id=844
> >
> > I don't think this is any actual diference; the const keyword is a nop
> > there. Issue 844 is just about the standard gratuitously including a
> > do-nothing keyword there.
> 
> Right. Keeping the qualifier here is harmless.

Oh, I read it backwards and thought we lacked the const. I'm in favor
of removing redundant/meaningless stuff in declarations. BTW all
instances of __restrict except one or two (pointer-to-__restrict) are
also of this type; they're meaningless in the declaration and perhaps
should be removed.

> >> inttypes.h
> >> ----------
> >> * wchar_t
> >> this symbol is exposed to the ISO C namespace
> >> AFAICT, this symbol is CX-shaded, and according to n1570 7.8.2.4p1,
> >> it seems to be intended that <stddef.h> be included to expose wchar_t
> >
> > Ah. This is problematic because functions declared in inttypes.h
> > require wchar_t to prototype. Of course a shadow name for the same
> > type can be defined (like how va_list is handled) but it's ugly...
> >
> >> wchar.h
> >> -------
> >> * FILE
> >> this symbol is exposed to the ISO C namespace
> >> AFAICT, this symbol is CX-shaded, and according to n1570 7.29.2.1p1,
> >> it seems to be intended that <stdio.h> be included to expose FILE
> >
> > Similar issue. It could be fixed with a shadot typedef or explicit
> > "struct _IO_FILE". The latter is ugly and something of a violation of
> > the abstraction, I think..
> 
> Indeed. I think ISO C should have exposed these data types.

I agree. Maybe we should change it though. I'll think about it. I know
there were tests (I think gcc fixincludes mess) flagging spurious
exposure of va_list as a bug, and in principle someone might decide to
do the same here, but maybe we should wait to make any change until
if/when there's a problem.

Rich

Re: Header conformance/improvements (part 2)

[Jens Gustedt]

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

Hello Rich,

On Sat, 24 Feb 2018 19:17:45 -0500 Rich Felker <dalias@libc.org> wrote:

> > > I don't think this is any actual diference; the const keyword is
> > > a nop there. Issue 844 is just about the standard gratuitously
> > > including a do-nothing keyword there.  
> > 
> > Right. Keeping the qualifier here is harmless.  
> 
> Oh, I read it backwards and thought we lacked the const. I'm in favor
> of removing redundant/meaningless stuff in declarations. BTW all
> instances of __restrict except one or two (pointer-to-__restrict) are
> also of this type; they're meaningless in the declaration and perhaps
> should be removed.

You are right concerning the validity of the interface, and I agree
that the const should be removed. It contains no valuable information
for the caller.

The restrict information is different. Restrict is a important
contract with the caller, and it is really a pitty that it cannot be
enforced. Nevertheless compilers can (and should) diagnose violations
of the restrict contract that it is able to deduce from the visible
code. So I think such information should not be removed from
interfaces.


> > > Ah. This is problematic because functions declared in inttypes.h
> > > require wchar_t to prototype. Of course a shadow name for the same
> > > type can be defined (like how va_list is handled) but it's ugly...
> > >  
>  [...]  
> > >
> > > Similar issue. It could be fixed with a shadot typedef or explicit
> > > "struct _IO_FILE". The latter is ugly and something of a
> > > violation of the abstraction, I think..  
> > 
> > Indeed. I think ISO C should have exposed these data types.  
> 
> I agree. Maybe we should change it though. I'll think about it. I know
> there were tests (I think gcc fixincludes mess) flagging spurious
> exposure of va_list as a bug, and in principle someone might decide to
> do the same here, but maybe we should wait to make any change until
> if/when there's a problem.

I agree, all C library header specifications in the standard should
force all types of their interfaces exposed. I note that as a
modification request for C2x, but I would not at all be sure that such
a change would be consensual, and such a change would still be only in
years to come.



Thanks
Jens

-- 
:: INRIA Nancy Grand Est ::: Camus ::::::: ICube/ICPS :::
:: ::::::::::::::: office Strasbourg : +33 368854536   ::
:: :::::::::::::::::::::: gsm France : +33 651400183   ::
:: ::::::::::::::: gsm international : +49 15737185122 ::
:: http://icube-icps.unistra.fr/index.php/Jens_Gustedt ::

[-- Attachment #2: Digitale Signatur von OpenPGP --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

Re: Header conformance/improvements (part 2)

[Rich Felker]

On Fri, Feb 23, 2018 at 02:20:49PM -0500, Rich Felker wrote:
> On Fri, Feb 23, 2018 at 01:32:49PM -0500, Daniel Sabogal wrote:
> > Here's a list of observations from musl's headers.
> 
> Thanks!
> 
> > tar.h
> > -----
> > * TSVTX
> > this constant is XSI-shaded
> > glibc exposes it with _XOPEN_SOURCE
> 
> tar.h is not governed by any modern standard. Not sure if there's any
> reason to change it. I feel like making it depend on FTMs is wrong.
> 
> > unistd.h
> > --------
> > * F_LOCK, F_TEST, F_TLOCK, F_ULOCK
> > these constants are XSI-shaded
> > glibc exposes them with _XOPEN_SOURCE
> 
> Indeed they go with lockf and should be in the #ifdef with it.
> 
> > stropts.h
> > ---------
> > * RPROTMASK
> > this constant is non-standard and not reserved
> > glibc exposes it with _GNU_SOURCE
> 
> Aside from the ioctl function, this is all deprecated/removed
> functionality, not governed by any profile of the standards we claim
> to support. Not sure if there's any reason to change it.
> 
> > signal.h
> > --------
> > * int sigqueue(pid_t, int, /* const */ union sigval);
> > harmless; it just doesn't reflect http://austingroupbugs.net/view.php?id=844
> 
> I don't think this is any actual diference; the const keyword is a nop
> there. Issue 844 is just about the standard gratuitously including a
> do-nothing keyword there.
> 
> > arch/*/bits/termios.h
> > ---------------------
> > * NLDLY, NL0, NL1
> > * CRDLY, CR0, CR1, CR2, CR3
> > * TABDLY, TAB0, TAB1, TAB2, TAB3
> > * BSDLY, BS0, BS1, FFDLY, FF0, FF1
> > these constants are XSI-shaded
> > (so are VTDLY, VT0 and VT1, but the prefix "V" is reserved by posix)
> > glibc exposes them with _XOPEN_SOURCE
> 
> This probably should be fixed; unfortunately it means moving some FTM
> logic into bits headers or refactoring.
> 
> > limits.h
> > --------
> > * PAGE_SIZE
> > * NL_LANGMAX
> > * NZERO
> > these constants are XSI-shaded
> > glibc exposes them with _XOPEN_SOURCE (except PAGE_SIZE)
> 
> OK, for PAGE_SIZE, the arch bits should be changed to define PAGESIZE
> instead of PAGE_SIZE and the top-level limits.h logic should be
> reversed to define PAGE_SIZE in terms of PAGESIZE. The others are
> defined in top-level file anyway and just need to be moved under
> proper FTMs.

Fixing all of the above, including tar.h.

> > sys/socket.h
> > ------------
> > * AF_* excluding AF_{INET,INET6,UNIX,UNSPEC}
> > * MSG_* excluding MSG_{CTRUNC,DONTROUTE,EOR,OOB,NOSIGNAL,PEEK,TRUNC,WAITALL}
> > * PF_*
> > * SCM_* excluding SCM_RIGHTS
> > * SO* excluding SOCK_{DGRAM,RAW,SEQPACKET,STREAM},
> > SO_{ACCEPTCONN,BROADCAST,DEBUG,DONTROUTE,ERROR,KEEPALIVE,LINGER,OOBINLINE,RCVBUF,RCVLOWAT,RCVTIMEO,REUSEADDR,SNDBUF,SNDLOWAT,SNDTIMEO,TYPE},
> > SOL_SOCKET, and SOMAXCONN
> > * CMSG_* excluding CMSG_{DATA,NXTHDR,FIRSTHDR}
> > these constants/macros are reserved by an XSI-shaded prefix
> > changing this might be too intrusive; glibc just exposes them
> 
> This is surprising. I doubt it would hurt to change, since little
> stuff builds with base POSIX profile anyway, but I'm not in a hurry to
> make changes here if not needed.

Leaving this for now. Will revisit if there's demand.

> > inttypes.h
> > ----------
> > * wchar_t
> > this symbol is exposed to the ISO C namespace
> > AFAICT, this symbol is CX-shaded, and according to n1570 7.8.2.4p1,
> > it seems to be intended that <stddef.h> be included to expose wchar_t
> 
> Ah. This is problematic because functions declared in inttypes.h
> require wchar_t to prototype. Of course a shadow name for the same
> type can be defined (like how va_list is handled) but it's ugly...
> 
> > wchar.h
> > -------
> > * FILE
> > this symbol is exposed to the ISO C namespace
> > AFAICT, this symbol is CX-shaded, and according to n1570 7.29.2.1p1,
> > it seems to be intended that <stdio.h> be included to expose FILE
> 
> Similar issue. It could be fixed with a shadot typedef or explicit
> "struct _IO_FILE". The latter is ugly and something of a violation of
> the abstraction, I think..

Leaving these alone too unless there's demand, for the reasons
discussed before.

Thanks for the reporting!

Rich