Using sys/types.h types in sys/socket.h

[Warner Losh]

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...

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?

To avoid namespace pollution, you'll need to include sys/_types.h use __uint32_t and __uintptr_t respectively. You'd also need
#if __BSD_VISIBLE
#ifndef _UINT32_T_DECLARED
typedef __uint32_t uint32_t;
#define _UINT32_T_DECLARED
#endif
and similar for __uintptr_t. thankfully, sys/_stdint.h already does this dance to avoid namespace pollution, so you just need a few lines at the top of socket.h to do this righ.

> +struct sf_hdtr_all {
> + struct sf_hdtr hdtr;
> + struct sf_hdtr_kq kq;
> +};
> +
> +/*
>  * Sendfile-specific flag(s)
>  */
> #define SF_NODISKIO     0x00000001
> #define SF_MNOWAIT 0x00000002
> #define SF_SYNC 0x00000004
> +#define SF_KQUEUE 0x00000008
> 
> #ifdef _KERNEL
> #define SFK_COMPAT 0x00000001
> 
> 
> ... now, uintptr_t upsets things, because we don't include sys/types.h
> before sys/socket.h.
> 
> The POSIX spec for sys/socket.h doesn't mention a dependency on
> sys/types.h and in fact says it should define a couple of types
> itself.
> 
> http://pubs.opengroup.org/onlinepubs/9699919799/basedefs/sys_socket.h.html
> 
> .. so, what suggestions do people have? I'd like to do this right and
> not cause header pollution.
> 
> Thanks!
> 
> 
> -a
> _______________________________________________
> freebsd-arch at freebsd.org mailing list
> http://lists.freebsd.org/mailman/listinfo/freebsd-arch
> To unsubscribe, send any mail to "freebsd-arch-unsubscribe at freebsd.org"