Linux-man Archive on lore.kernel.org
 help / color / Atom feed
* f_owner_ex vs. POSIX
@ 2019-08-29 15:50 Eric Blake
  2019-09-01 13:23 ` Michael Kerrisk (man-pages)
  0 siblings, 1 reply; 5+ messages in thread
From: Eric Blake @ 2019-08-29 15:50 UTC (permalink / raw)
  To: glibc list, mtk.manpages; +Cc: linux-man

[-- Attachment #1.1: Type: text/plain, Size: 3863 bytes --]

The Austin Group is considering standardizing a subset of the Linux
fcntl(F_GETOWN_EX), because of its ability to overcome the limitation
that fcntl(F_GETOWN) must fail for some valid pids if pid_t is permitted
to be wider than int (whether or not Linux ever reaches a point where
pid_t is wider than int, POSIX did not want to make that restriction on
other implementations).  See http://austingroupbugs.net/view.php?id=1274

However, we've run into a minor issue which implies that man-pages
and/or glibc is buggy:

The man page for fcntl() (as of Fedora 30 man-pages-4.16-4.fc30) states:

                  struct f_owner_ex {
                      int   type;
                      pid_t pid;
                  };

but in the headers under /usr/include, there are two different
definitions, which raises the question on what the real type of 'type'
should be:

/usr/include/asm-generic/fcntl.h (from kernel-headers-5.2.9-200.fc30):
struct f_owner_ex {
        int     type;
        __kernel_pid_t  pid;
};

/usr/include/bits/fcntl-linux.h (from glibc-headers-2.29-15.fc30):

struct f_owner_ex
  {
    enum __pid_type type;       /* Owner type of ID.  */
    __pid_t pid;                /* ID of owner.  */
  };


Note that an enum instead of an int matters as to whether this will
complain when compiled:
struct f_owner_ex s;
int *foo = &s.type;

Therefore, we want to confirm whether requiring the eventual POSIX
definition to use enum f_pid_type (as currently worded in
austingroupbugs.net/view.php?id=1274#c4536) is okay (in which case,
there is a bug in the man page for documenting int instead of enum
f_pid_type), or if POSIX should not bother defining enum f_pid_type (and
instead just provide F_OWNER_PID and F_OWNER_PGRP as macros) with
f_owner_ex being defined with an int (in which case, the glibc <fcntl.h>
header needs a change to use int, and the Austin Group proposal needs to
be tweaked to match).

Note that the use of an enum in a public struct makes that struct
dependent on ABI issues (if the library is compiled with one set of
compiler flags where enums occupy the space of 'int', but an application
compiles with a different set of flags where an enum occupies only the
space of 'char', this could result in the application being unable to
correctly call into libc), if that helps sway the decision on which of
the two projects needs to change.  However, the exact layout of the
struct and any padding space was not deemed to be a showstopper (that
is, similar to struct stat, the standard intends only to require that at
least two members be present in f_owner_ex without any further
restrictions on what layout those two members occupy).

A side note was also raised during discussion: POSIX already
standardizes the type idtype_t for use in waitid(), and on Linux, we
happen to have P_PID==F_OWNER_PID==1 and P_PGID==F_OWNER_PGRP==2 (which
are the only values that POSIX is considering adding), which on the
surface looks like unnecessary duplication.  So at one point, the
question was raised whether POSIX should reuse the existing idtype_t
instead of inventing something new for f_owner_ex.  However, it was then
pointed out that idtype_t also includes P_ALL (which on Linux is 0), and
that Linux uses F_OWNER_TID==0 as an extension to what POSIX would
require, but since Linux' F_OWNER_TID semantics for F_SETOWN_EX are not
the same semantics as P_ALL in waitid(); furthermore, <fcntl.h> has free
reign to add more F_* into the namespace but not P_*, where reuse of the
idtype_t type would then require dragging in the <sys/wait.h> header
just to populate f_owner_ex.  Thus, this reuse of types was deemed
unpalatable.

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: f_owner_ex vs. POSIX
  2019-08-29 15:50 f_owner_ex vs. POSIX Eric Blake
@ 2019-09-01 13:23 ` Michael Kerrisk (man-pages)
  2019-09-02 13:44   ` Florian Weimer
  0 siblings, 1 reply; 5+ messages in thread
From: Michael Kerrisk (man-pages) @ 2019-09-01 13:23 UTC (permalink / raw)
  To: Eric Blake, glibc list; +Cc: mtk.manpages, linux-man, Florian Weimer

[Explicitly CCing Florian Weimer, since he may have some thoughts.]

Hello Eric,

On 8/29/19 5:50 PM, Eric Blake wrote:
> The Austin Group is considering standardizing a subset of the Linux
> fcntl(F_GETOWN_EX), because of its ability to overcome the limitation
> that fcntl(F_GETOWN) must fail for some valid pids if pid_t is permitted
> to be wider than int (whether or not Linux ever reaches a point where
> pid_t is wider than int, POSIX did not want to make that restriction on
> other implementations).  See http://austingroupbugs.net/view.php?id=1274
> 
> However, we've run into a minor issue which implies that man-pages
> and/or glibc is buggy:
> 
> The man page for fcntl() (as of Fedora 30 man-pages-4.16-4.fc30) states:
> 
>                    struct f_owner_ex {
>                        int   type;
>                        pid_t pid;
>                    };
> 
> but in the headers under /usr/include, there are two different
> definitions, which raises the question on what the real type of 'type'
> should be:
> 
> /usr/include/asm-generic/fcntl.h (from kernel-headers-5.2.9-200.fc30):
> struct f_owner_ex {
>          int     type;
>          __kernel_pid_t  pid;
> };
> 
> /usr/include/bits/fcntl-linux.h (from glibc-headers-2.29-15.fc30):
> 
> struct f_owner_ex
>    {
>      enum __pid_type type;       /* Owner type of ID.  */
>      __pid_t pid;                /* ID of owner.  */
>    };
> 
> 
> Note that an enum instead of an int matters as to whether this will
> complain when compiled:
> struct f_owner_ex s;
> int *foo = &s.type;
> 
> Therefore, we want to confirm whether requiring the eventual POSIX
> definition to use enum f_pid_type (as currently worded in
> austingroupbugs.net/view.php?id=1274#c4536) is okay (in which case,
> there is a bug in the man page for documenting int instead of enum
> f_pid_type), or if POSIX should not bother defining enum f_pid_type (and
> instead just provide F_OWNER_PID and F_OWNER_PGRP as macros) with
> f_owner_ex being defined with an int (in which case, the glibc <fcntl.h>
> header needs a change to use int, and the Austin Group proposal needs to
> be tweaked to match).

So, a little background.

The kernel feature was added in Linux 2.6.32, which was tagged in
December 2009.

I added the manual page text at the start of October 2009, based on
the types used in the kernel structure.

By chance, the glibc structure definition was added at the end of the 
same month. (I do not recall, but I suspect that I did not notice
the glibc addition.)

I do not know what the rationale was for the addition of the 'enum',
and it wouldn't surprise me if there was no public discussion about
it. The use of an 'enum' strikes me as a slightly odd decision (given
that the kernel uses 'int') but, related to your point below, there
is precedent in, for example, the use of an 'enum' for 'idtype_t' in
waitid() inside glibc, while the kernel type for the argument in
the underlying system call is 'int'.

> Note that the use of an enum in a public struct makes that struct
> dependent on ABI issues (if the library is compiled with one set of
> compiler flags where enums occupy the space of 'int', but an application
> compiles with a different set of flags where an enum occupies only the
> space of 'char', this could result in the application being unable to
> correctly call into libc), if that helps sway the decision on which of
> the two projects needs to change.  However, the exact layout of the
> struct and any padding space was not deemed to be a showstopper (that
> is, similar to struct stat, the standard intends only to require that at
> least two members be present in f_owner_ex without any further
> restrictions on what layout those two members occupy).
> 
> A side note was also raised during discussion: POSIX already
> standardizes the type idtype_t for use in waitid(), and on Linux, we
> happen to have P_PID==F_OWNER_PID==1 and P_PGID==F_OWNER_PGRP==2 (which
> are the only values that POSIX is considering adding), which on the
> surface looks like unnecessary duplication.  So at one point, the
> question was raised whether POSIX should reuse the existing idtype_t
> instead of inventing something new for f_owner_ex.  However, it was then
> pointed out that idtype_t also includes P_ALL (which on Linux is 0), and
> that Linux uses F_OWNER_TID==0 as an extension to what POSIX would
> require, but since Linux' F_OWNER_TID semantics for F_SETOWN_EX are not
> the same semantics as P_ALL in waitid(); furthermore, <fcntl.h> has free
> reign to add more F_* into the namespace but not P_*, where reuse of the
> idtype_t type would then require dragging in the <sys/wait.h> header
> just to populate f_owner_ex.  Thus, this reuse of types was deemed
> unpalatable.

I'm agnostic on whether it's the manual page of glibc that should
be fixed. The ABI issues that you note above are unfortunate, of
course. (Do they not suggest that standard really should use 'int'?)

Cheers,

Michael

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: f_owner_ex vs. POSIX
  2019-09-01 13:23 ` Michael Kerrisk (man-pages)
@ 2019-09-02 13:44   ` Florian Weimer
  2019-09-03  3:51     ` Rich Felker
  0 siblings, 1 reply; 5+ messages in thread
From: Florian Weimer @ 2019-09-02 13:44 UTC (permalink / raw)
  To: Michael Kerrisk \(man-pages\); +Cc: Eric Blake, glibc list, linux-man

* Michael Kerrisk:

> I do not know what the rationale was for the addition of the 'enum',
> and it wouldn't surprise me if there was no public discussion about
> it. The use of an 'enum' strikes me as a slightly odd decision (given
> that the kernel uses 'int') but, related to your point below, there
> is precedent in, for example, the use of an 'enum' for 'idtype_t' in
> waitid() inside glibc, while the kernel type for the argument in
> the underlying system call is 'int'.

There is also the issue of -fshort-enum.  Some people probably expect
that they can use that option and still use glibc headers.

I do not have any inside knowledge why things are like they are.
Presumably we can switch the type member to int.

Thanks,
Florian

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: f_owner_ex vs. POSIX
  2019-09-02 13:44   ` Florian Weimer
@ 2019-09-03  3:51     ` Rich Felker
  2019-09-09 15:15       ` Eric Blake
  0 siblings, 1 reply; 5+ messages in thread
From: Rich Felker @ 2019-09-03  3:51 UTC (permalink / raw)
  To: Florian Weimer
  Cc: Michael Kerrisk (man-pages), Eric Blake, glibc list, linux-man

On Mon, Sep 02, 2019 at 03:44:53PM +0200, Florian Weimer wrote:
> * Michael Kerrisk:
> 
> > I do not know what the rationale was for the addition of the 'enum',
> > and it wouldn't surprise me if there was no public discussion about
> > it. The use of an 'enum' strikes me as a slightly odd decision (given
> > that the kernel uses 'int') but, related to your point below, there
> > is precedent in, for example, the use of an 'enum' for 'idtype_t' in
> > waitid() inside glibc, while the kernel type for the argument in
> > the underlying system call is 'int'.
> 
> There is also the issue of -fshort-enum.  Some people probably expect
> that they can use that option and still use glibc headers.
> 
> I do not have any inside knowledge why things are like they are.
> Presumably we can switch the type member to int.

I'm strongly in favor of switch to int. enum types are an
ABI/compatibility nightmare and have little purpose (unlike enum
constants which are actually useful).

Rich

^ permalink raw reply	[flat|nested] 5+ messages in thread

* Re: f_owner_ex vs. POSIX
  2019-09-03  3:51     ` Rich Felker
@ 2019-09-09 15:15       ` Eric Blake
  0 siblings, 0 replies; 5+ messages in thread
From: Eric Blake @ 2019-09-09 15:15 UTC (permalink / raw)
  To: Rich Felker, Florian Weimer
  Cc: Michael Kerrisk (man-pages), glibc list, linux-man, Adhemerval Zanella

[-- Attachment #1.1: Type: text/plain, Size: 1424 bytes --]

On 9/2/19 10:51 PM, Rich Felker wrote:
> On Mon, Sep 02, 2019 at 03:44:53PM +0200, Florian Weimer wrote:
>> * Michael Kerrisk:
>>
>>> I do not know what the rationale was for the addition of the 'enum',
>>> and it wouldn't surprise me if there was no public discussion about
>>> it. The use of an 'enum' strikes me as a slightly odd decision (given
>>> that the kernel uses 'int') but, related to your point below, there
>>> is precedent in, for example, the use of an 'enum' for 'idtype_t' in
>>> waitid() inside glibc, while the kernel type for the argument in
>>> the underlying system call is 'int'.
>>
>> There is also the issue of -fshort-enum.  Some people probably expect
>> that they can use that option and still use glibc headers.
>>
>> I do not have any inside knowledge why things are like they are.
>> Presumably we can switch the type member to int.
> 
> I'm strongly in favor of switch to int. enum types are an
> ABI/compatibility nightmare and have little purpose (unlike enum
> constants which are actually useful).

I'm also in favor of 'int' (but not the 'int32_t' proposal mentioned in
note 4538).  Does anyone volunteer to write up the glibc patch, while I
report back to the Austin Group that 'int' is the preferred type for
standardization?

-- 
Eric Blake, Principal Software Engineer
Red Hat, Inc.           +1-919-301-3226
Virtualization:  qemu.org | libvirt.org


[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

^ permalink raw reply	[flat|nested] 5+ messages in thread

end of thread, back to index

Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-08-29 15:50 f_owner_ex vs. POSIX Eric Blake
2019-09-01 13:23 ` Michael Kerrisk (man-pages)
2019-09-02 13:44   ` Florian Weimer
2019-09-03  3:51     ` Rich Felker
2019-09-09 15:15       ` Eric Blake

Linux-man Archive on lore.kernel.org

Archives are clonable:
	git clone --mirror https://lore.kernel.org/linux-man/0 linux-man/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 linux-man linux-man/ https://lore.kernel.org/linux-man \
		linux-man@vger.kernel.org linux-man@archiver.kernel.org
	public-inbox-index linux-man


Newsgroup available over NNTP:
	nntp://nntp.lore.kernel.org/org.kernel.vger.linux-man


AGPL code for this site: git clone https://public-inbox.org/ public-inbox