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

[Bruce Evans]

On Wed, 18 Dec 2013, Adrian Chadd wrote:

> Ok, how about this:
>
> Index: sys/sys/socket.h
> ===================================================================
> --- sys/sys/socket.h    (revision 259475)
> +++ sys/sys/socket.h    (working copy)
> @@ -84,6 +84,16 @@
> #endif
> #endif
>
> +#ifndef _UINT32_T_DECLARED
> +#define        _UINT32_T_DECLARED
> +typedef __uint32_t     uint32_t;
> +#endif
> +
> +#ifndef _UINTPTR_T_DECLARED
> +#define _UINTPTR_T_DECLARED
> +typedef __uintptr_t    uintptr_t;
> +#endif
> +
> /*
>  * Types
>  */

This seems to be correct, except the tab after the second #define is
corrupt.  Actually, all the tabs are corrupt, but the first #define
apparently started with a tab whose corruption made a larger mess.

imp@ said, in a message that should have been killfiled due to top posting,
that this should be under __BSD_VISIBLE.  That isn't strictly necessary,
since POSIX allows names ending with _t, and it isn't very important for
avoiding pollution since there aren't very many of them.

> @@ -577,11 +587,27 @@
> };
>
> /*
> + * sendfile(2) kqueue information
> + */
> +struct sf_hdtr_kq {
> +       int kq_fd;              /* kq fd to post completion events on */
> +       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?) */
> +};

kq_fd is duplicated.

All of the indentation is wrong and has corrupt tabs, except the first
level only has corrupt tabs.

This should be:

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?) */
};

I strongly disagree with indenting everything N extra levels (where N > 1;
N = 2 here for the name fields) to line up the fields.  I disagree with
indenting the name fields by 1 space extra to line them up after adding
a '*' before a few fields.  It's hard enough to keep the style consistent
without following this fancy style.

> +
> +struct sf_hdtr_all {
> +       struct sf_hdtr hdtr;
> +       struct sf_hdtr_kq kq;
> +};

Here an extra level would be reasonable since all fields need it, except
it would leave less space for comments.  Comments are needed for these
fields more than for most, since there are none for the struct itself.

> +
> +/*
>  * Sendfile-specific flag(s)
>  */

Older style bugs:
- sendfile(2) is not spelled consistently
- the parentheses around 's' are more bogus than before because it is
   even more obvious that there is more than 1 flag here.  This was
   non-bogus in FreeBSD-5 where the only flag here was SF_NODISKIO.
- this and some other comments are too verbose (block comments are not needed)
- the style rules for block comments are stricter, but are not followed.
   The sentence fragment in this comment is not a full sentence, and the
   paragraph in this comment is not filled to make it look like a real
   paragraph (filling would also make the sentence fragment look like a
   sentence).  I like to write comments about flags uniformly as
   "/* Flags for foo: */".

> #define        SF_NODISKIO     0x00000001
> #define        SF_MNOWAIT      0x00000002
> #define        SF_SYNC         0x00000004
> +#define        SF_KQUEUE       0x00000008

The names are unsorted.  This seems to be unfortunately necessary for binary
compatibility.

>

This blank line breaks the scope of the comment.

> #ifdef _KERNEL
> #define        SFK_COMPAT      0x00000001
>

Bruce