Date: Tue, 9 Sep 2014 03:13:45 -0700 From: yaneurabeya@gmail.com To: Konstantin Belousov <kostikbel@gmail.com> Cc: "freebsd-arch@FreeBSD.org Arch" <freebsd-arch@freebsd.org> Subject: Re: [RFC] Add __arraycount from NetBSD to sys/cdefs.h Message-ID: <AB0899A0-02B1-4952-8F8E-D4AB4AADED21@gmail.com> In-Reply-To: <20140909083333.GA2737@kib.kiev.ua> References: <CAGHfRMBMPra5YXDn0e83dpVxwnarg3DL8o31xr7DhWv%2BVXskTg@mail.gmail.com> <8D279BDC-7D40-4750-8DA7-A4535DD2E458@bsdimp.com> <146C3E96-7461-4D30-8B7F-5E20F72CF061@gmail.com> <20140909083333.GA2737@kib.kiev.ua>
next in thread | previous in thread | raw e-mail | index | archive | help
--Apple-Mail=_C7E4E3CD-90FE-4FEB-A167-21EC9CF454F4
Content-Transfer-Encoding: quoted-printable
Content-Type: text/plain;
charset=windows-1252
Hi kib@!
On Sep 9, 2014, at 1:33, Konstantin Belousov <kostikbel@gmail.com> =
wrote:
> On Mon, Sep 08, 2014 at 09:29:54AM -0700, yaneurabeya@gmail.com wrote:
>> On Sep 3, 2014, at 21:17, Warner Losh <imp@bsdimp.com> wrote:
>>=20
>>> On Sep 3, 2014, at 9:45 PM, Garrett Cooper <yaneurabeya@gmail.com> =
wrote:
>>>=20
>>>> Hi all,
>>>> In order to ease porting code and reduce divergence with NetBSD
>>>> when importing code (a large chunk of which for me are tests), I =
would
>>>> like to move nitems to sys/cdefs.h and alias __arraycount to =
nitems.
>>>> Here's the __arraycount #define in lib/libnetbsd/sys/cdefs.h:
>>>>=20
>>>> 44 /*
>>>> 45 * Return the number of elements in a statically-allocated =
array,
>>>> 46 * __x.
>>>> 47 */
>>>> 48 #define __arraycount(__x) (sizeof(__x) / sizeof(__x[0]))
>>>>=20
>>>> Here's the nitems #define in sys/sys/param.h:
>>>>=20
>>>> 277 #define nitems(x) (sizeof((x)) / sizeof((x)[0]))
>>>>=20
>>>> sys/cdefs.h gets pulled in automatically with sys/param.h, so
>>>> anything using nitems will continue to function like before (see =
below
>>>> for more details). I've attached a patch which addresses all =
hardcoded
>>>> definitions in the tree added by FreeBSD developers.
>>>> If there aren't any major concerns with my proposed change, I'll
>>>> put it up for review on Phabricator.
>>>> Thank you!
>>>> -Garrett
>>>>=20
>>>> $ cat cdefs_pound_define.c
>>>> #include <sys/param.h>
>>>>=20
>>>> #ifdef _SYS_CDEFS_H_
>>>> #warning "sys/cdefs.h has been included"
>>>> #endif
>>>> $ cc -c cdefs_pound_define.c
>>>> cdefs_pound_define.c:4:2: warning: "sys/cdefs.h has been included" =
[-W#warnings]
>>>> #warning "sys/cdefs.h has been included"
>>>> ^
>>>> 1 warning generated.
>>>> $ cc -D_KERNEL -c cdefs_pound_define.c
>>>> cdefs_pound_define.c:4:2: warning: "sys/cdefs.h has been included" =
[-W#warnings]
>>>> #warning "sys/cdefs.h has been included"
>>>> ^
>>>> 1 warning generated.
>>>> $ gcc -c cdefs_pound_define.c
>>>> cdefs_pound_define.c:4:2: warning: #warning "sys/cdefs.h has been =
included"
>>>> $ gcc -D_KERNEL -c cdefs_pound_define.c
>>>> cdefs_pound_define.c:4:2: warning: #warning "sys/cdefs.h has been =
included?
>>>=20
>>> I wouldn?t bother changing the nitems #define. There?s no need, =
really, to do that.
>>=20
>> Rethinking my proposal, I agree. I had lofty hopes for unifying the =
macros, but the functional duplication (1 line) is harmless.
>>=20
>>> I?d also be more inclined to believe the test if you tested what the =
thing does rather than test for an artificial, implementation defined =
side effect.
>>=20
>> Sure. I provided a lazy proof instead of a full proof :).
>>=20
>>> But honestly the amount of duplication saved here is rather tiny?
>>=20
>> Indeed! Thank you for the input :) ? I?ve attached a new patch which =
doesn?t disturb nitems or sys/param.h.
>=20
> Since this is seriously discussed on arch@, I think it is worth look
> at the following:
>=20
> git grep -e 'sizeof.*/sizeof' -- | grep -v '^contrib' | grep =
'#.*define' | wc -l
> 118
> These are only defines, not uses, not counting direct use of
> sizeof(x)/sizeof(x[0]) in the code.
It=92s a bit less painful if you omit cddl, crypto, and gnu:
$ git grep -e 'sizeof.*/sizeof' -- | egrep -v =
'^(cddl|contrib|crypto|gnu)' | grep '#.*define' | wc -l
105
> Not all of them are for equiv macros, but significant part repeats =
ntimes()
> in variations. At least Linuxish ARRAY_SIZE/DRM_ARRAY_SIZE is much =
more
> popular than __arraysize() which appears three (!) times.
Hmm.. that=92s a bit surprising.
> I do not see any reason to pollute central header cdefs.h with =
sparcely
> used macro for some compat code.
This is one compelling reason =97 it=92s used 225 times in the test code =
in NetBSD, spanning 92 files, and 15 times spanning 10 files in the rump =
kernel.
$ grep -rl __arraycount netbsd/src/tests/ | wc -l; grep -r __arraycount =
netbsd/src/tests/ | wc -l
92
225
$ grep -rl __arraycount netbsd/src/sys/rump | wc -l; grep -r =
__arraycount netbsd/src/sys/rump | wc -l
10
15
My goal is to pull in as much test code from NetBSD that I=92ve done on =
my github fork and apply it to FreeBSD with as little deviations as =
possible, wherever possible. The less we deviate and can push back and =
forth between FreeBSD and NetBSD, the easier it will be in the long run =
for both parties to develop and maintain test code and other code from =
NetBSD.
That being said, there might be a compromise that could be struck where =
the compat code can stay confined to libnetbsd. If I change the code =
bsd.test.mk or contributed code to explicitly use -include or -I, then =
point to the lib/libnetbsd/sys/cdefs.h header, then this could become a =
non-issue:
-include filename
Adds an implicit #include into the predefines buffer which is =
read
before the source file is preprocessed.
-Idirectory
Add the specified directory to the search path for include =
files.
Does that seem like an appropriate solution?
Thanks!
-Garrett
PS For what it=92s worth, nitems use has caught on like wildfire (it =
beats out __arraycount and the ad hoc sizeof):
$ git grep -e nitems -- | egrep -v '^(cddl|contrib|crypto|gnu)' | wc -l
551
--Apple-Mail=_C7E4E3CD-90FE-4FEB-A167-21EC9CF454F4
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment;
filename=signature.asc
Content-Type: application/pgp-signature;
name=signature.asc
Content-Description: Message signed with OpenPGP using GPGMail
-----BEGIN PGP SIGNATURE-----
Comment: GPGTools - https://gpgtools.org
iQEcBAEBCgAGBQJUDtLZAAoJEMZr5QU6S73e4xUH/jjB6W8ng18g5kMRZ2K/6ySi
WPvmKZE3FTpAXzKSFtT5OJo9T1rqfV4+YhsncXkvUhtlLCDuyDjJwqTuWKRdhg47
zDRHSjVOEMH7InWN+OzaI8FfDC1TYI++hKfMHcdUYs8zgLxOWYTrKV/ZPgBFRDE0
PDWA4E+c4956Jaqd2Ixyrb9vY1S0gaUW2Gq1yrxNf9+66UXyWCIfZ0qRUEDPcU65
MtUMwJ6cHcyJZ9Yso144GSZXAZQOn/PNe1/+3AroJj/9DzYQGj39XlgNLBHdVzPs
dU2y3oMFkvgKNGM5c2REqGZJbznE9JBVutUthx60Vn0qjATqsnvh09elo6l+tTs=
=nxsF
-----END PGP SIGNATURE-----
--Apple-Mail=_C7E4E3CD-90FE-4FEB-A167-21EC9CF454F4--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AB0899A0-02B1-4952-8F8E-D4AB4AADED21>