Date: Wed, 8 Jan 2014 15:48:15 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Warner Losh <imp@bsdimp.com> Cc: Adrian Chadd <adrian@FreeBSD.org>, Bruce Evans <bde@FreeBSD.org>, "freebsd-arch@freebsd.org" <freebsd-arch@FreeBSD.org> Subject: Re: Using sys/types.h types in sys/socket.h Message-ID: <20140108152632.L969@besplex.bde.org> In-Reply-To: <5821DA91-E9C7-4D1A-A108-63E74CFF1FC5@bsdimp.com> References: <CAJ-Vmo=MWPQWfP9duWPPwaKee5Zp9Gemj3GKqE8=bxkjn_1YYA@mail.gmail.com> <9C1291B5-215B-440E-B8B0-6308840F755C@bsdimp.com> <CAJ-Vmom3FQOb1ZQTwbmbSiT90N6tmKOoPg8JPQAapkJg2TXqFA@mail.gmail.com> <5821DA91-E9C7-4D1A-A108-63E74CFF1FC5@bsdimp.com>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 7 Jan 2014, Warner Losh wrote:
> On Jan 7, 2014, at 2:20 PM, Adrian Chadd wrote:
>
>> On 17 December 2013 07:42, Warner Losh <imp@bsdimp.com> wrote:
>>> The tl;dr version: use sys/_types.h and use the usual conventions to avoid namespace pollution. I read Bruce's reply, and I think I'm saying the same things he is...
>>
>> Yup, I've done that bit.
>>
>>> Warner
>>>
>>> On Dec 17, 2013, at 1:23 AM, Adrian Chadd wrote:
>>>
>>>> Hi,
>>>>
>>>> I have a patch to implement some new sendfile functionality, but this
>>>> involves adding stuff to sys/socket.h:
>>>>
>>>> Index: sys/sys/socket.h
>>>> ===================================================================
>>>> --- sys/sys/socket.h (revision 258883)
>>>> +++ sys/sys/socket.h (working copy)
>>>> @@ -577,11 +577,27 @@
>>>> };
>>>>
>>>> /*
>>>> + * sendfile(2) kqueue information
>>>> + */
>>>> +struct sf_hdtr_kq {
>>>> + int kq_fd; /* kq fd to post completion events on */
>>>> + uint32_t kq_flags; /* extra flags to pass in */
>>>> + void *kq_udata; /* user data pointer */
>>>> + uintptr_t kq_ident; /* ident (from userland?) */
>>>> +};
>>>
>>> This is a terrible interface. Or I'd say that the ordering of the elements in this structure is suboptimal. Having the uint32_t in the middle like that causes badness. Guess not much can be done about that given that fd must be an int, eh?
>>
>> I'm open to an alternate interface that doesn't require new syscalls
>> or syscall modification. Yeah, the definition of an fd here is an
>> 'int'. I could always turn it into an int32_t to fix the size but
>> would that be "ok" ?
No.
> I'm not sure it is worth fixing in this way, since it is counter to all the other uses of fd in the system, so maybe this is the best we can have given those constraints...
This API has many design errors and style bugs, but ints for file
are the least serious of them. int is the same as int32_t on all supported
arches, so the uint32_t is not in the middle, but gives perfect packing to
a 64-bit boundary of the int before it. In fact, everything is packed:
32-bit arches: 64-bit arches:
32-bit int 32-bit int
32-bit u_int 32-bit u_int
32-bit pointer 64-bit pointer
32-bit u_int holding ptr 64-bit u_int holding pointer
Style(9) specifies sorting by size first (it actually mean by alignment
first). That is not very easy since the size^Walignment of typedefed
types should be opaque. In practice, assuming what it is mostly gives
correct results. It gives exactly the opposite of the above:
N-bit u_int holding ptr
M-bit pointer /* assume M <= N and alignment == size */
32-bit u_int (can spell it u_int, not uint32_t, to pack better with int)
32-bit int /* assume plain int gives this */
Types that usually have the same size should be sorted on rank, although
style(9) doesn't say this (see the C99 standard for a formal definition
of 'rank'). This gives int before long, although on i386 int has the
same size as long. It takes an exotic unsupported arch for types of
smaller rank to have larger size/ alignment. This also gives int
before u_int, although int has the same size as u_int on all supported
arches.
After assuming that plain int has >= 32 bits, we can assume this for the
flags arg too, to get at least 32 bits from it. This minimizes assumptions
mad in packing. Using int32_t instead of int for file descriptors would
be a slightly worse way of assuming that ints are never expanded from 32
bits.
Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140108152632.L969>