Message-ID: <CAKCAbMg_eRCsD-HYmryL8XEuZcaM1Qdfp4XD85QKT6To+h3QcQ@mail.gmail.com>
Date:   Tue, 4 May 2021 14:37:20 -0400
From:   Zack Weinberg <zackw@...ix.com>
To:     Greg KH <gregkh@...uxfoundation.org>
Cc:     "Alejandro Colomar (man-pages)" <alx.manpages@...il.com>,
        Alexei Starovoitov <alexei.starovoitov@...il.com>,
        "Michael Kerrisk (man-pages)" <mtk.manpages@...il.com>,
        linux-man <linux-man@...r.kernel.org>,
        LKML <linux-kernel@...r.kernel.org>,
        glibc <libc-alpha@...rceware.org>, GCC <gcc-patches@....gnu.org>,
        bpf <bpf@...r.kernel.org>,
        David Laight <David.Laight@...lab.com>,
        Joseph Myers <joseph@...esourcery.com>
Subject: Re: [RFC v2] bpf.2: Use standard types and attributes

On Tue, May 4, 2021 at 12:06 PM Greg KH <gregkh@...uxfoundation.org> wrote:
> On Tue, May 04, 2021 at 05:53:29PM +0200, Alejandro Colomar (man-pages) wrote:
> > On 5/4/21 4:24 PM, Greg KH wrote:
> > > I agree, the two are not the same type at all, this change should not be
> > > accepted.
> >
> > I get that in the kernel you don't use the standard fixed-width types (with
> > some exceptions), probably not to mess with code that relies on <stdint.h>
> > not being included (I hope there's not much code that relies on this in
> > 2021, but who knows).
> >
> > But, there is zero difference between these types, from the point of view of
> > the compiler.  There's 100% compatibility between those types, and you're
> > able to mix'n'match them.  See some example below.
...
> There's a very old post from Linus where he describes the difference
> between things like __u32 and uint32_t.  They are not the same, they
> live in different namespaces, and worlds, and can not always be swapped
> out for each other on all arches.
>
> Dig it up if you are curious, but for user/kernel apis you HAVE to use
> the __uNN and can not use uintNN_t variants, so don't try to mix/match
> them, it's good to just follow the kernel standard please.
...
> Nacked-by: Greg Kroah-Hartman <gregkh@...uxfoundation.org>

Speaking from the C library's perspective, I'm going to push back
pretty hard on this NAK, for several reasons.

First, this is a proposed change to the manpages, not the headers
themselves.  Manpage documentation of C structs is *not* expected to
match the actual declaration in the headers.  The documented field
type is usually assignment-compatible with the actual type, but not
always.  There's no guarantee whatsoever that the fields are in the
same order as the header, or that the listed set of fields is
complete.

I would say that as long as any value of type __u32 can be stored in a
variable of type uint32_t without data loss, and vice versa, there is
no reason why manpages should *have to* use __u32 in preference to
uint32_t, and that in the absence of such a reason, the standard type
should be used.

Second, it's true that __u32 and uint32_t are in different namespaces,
and it may well be necessary for uapi <linux/*.h> headers to use the
__uNN names in order to preserve the C standard's distinction between
the program and the implementation, but that's *not* a reason for
documentation aimed at writers of user-space programs to use the
__uNN names.  In fact, it is exactly the opposite!  User space program
authors should, all else equal, be *discouraged* from using the __uNN
names, and avoiding their use in manpages is one way to do that.

Third, if there does in fact exist a situation where __uNN and
uintNN_t are *not* assignment compatible, THAT IS A BUG IN THE KERNEL.
Frankly, it would be such a catastrophic bug that I think Linus has to
have been *wrong*.  We would have noticed the problems long ago if he
were right.

I'm going to have to ask you to produce hard evidence for your claim
that __uNN and uintNN_t are not (always) assignment compatible, and
hard evidence why that can't be fixed within the kernel, or else
withdraw your objection.

zw

Powered by blists - more mailing lists