From owner-freebsd-arch@FreeBSD.ORG Thu Dec 19 05:51:23 2013 Return-Path: Delivered-To: freebsd-arch@FreeBSD.org Received: from mx1.freebsd.org (mx1.freebsd.org [8.8.178.115]) (using TLSv1 with cipher ADH-AES256-SHA (256/256 bits)) (No client certificate requested) by hub.freebsd.org (Postfix) with ESMTPS id 997D3EE9; Thu, 19 Dec 2013 05:51:23 +0000 (UTC) Received: from mail109.syd.optusnet.com.au (mail109.syd.optusnet.com.au [211.29.132.80]) by mx1.freebsd.org (Postfix) with ESMTP id 1D0A41D01; Thu, 19 Dec 2013 05:51:22 +0000 (UTC) Received: from c122-106-144-87.carlnfd1.nsw.optusnet.com.au (c122-106-144-87.carlnfd1.nsw.optusnet.com.au [122.106.144.87]) by mail109.syd.optusnet.com.au (Postfix) with ESMTPS id B7649D60C80; Thu, 19 Dec 2013 16:20:21 +1100 (EST) Date: Thu, 19 Dec 2013 16:20:20 +1100 (EST) From: Bruce Evans X-X-Sender: bde@besplex.bde.org To: Adrian Chadd Subject: Re: Using sys/types.h types in sys/socket.h In-Reply-To: Message-ID: <20131219154839.T23018@besplex.bde.org> References: <9C1291B5-215B-440E-B8B0-6308840F755C@bsdimp.com> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed X-Optus-CM-Score: 0 X-Optus-CM-Analysis: v=2.1 cv=HZAtEE08 c=1 sm=1 tr=0 a=p/w0leo876FR0WNmYI1KeA==:117 a=PO7r1zJSAAAA:8 a=n4PiHyf99_4A:10 a=kj9zAlcOel0A:10 a=JzwRw_2MAAAA:8 a=zprIhm_aaxwA:10 a=rF_X41fraC8vsB5qnxEA:9 a=CjuIK1q_8ugA:10 Cc: "freebsd-arch@freebsd.org" X-BeenThere: freebsd-arch@freebsd.org X-Mailman-Version: 2.1.17 Precedence: list List-Id: Discussion related to FreeBSD architecture List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Thu, 19 Dec 2013 05:51:23 -0000 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