Date: Wed, 10 Sep 2014 00:03:29 +0300 From: Konstantin Belousov <kostikbel@gmail.com> To: yaneurabeya@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: <20140909210329.GE2737@kib.kiev.ua> In-Reply-To: <AB0899A0-02B1-4952-8F8E-D4AB4AADED21@gmail.com> 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> <AB0899A0-02B1-4952-8F8E-D4AB4AADED21@gmail.com>
next in thread | previous in thread | raw e-mail | index | archive | help
--WEBrq8xPpGVQldHe Content-Type: text/plain; charset=us-ascii Content-Disposition: inline Content-Transfer-Encoding: quoted-printable On Tue, Sep 09, 2014 at 03:13:45AM -0700, yaneurabeya@gmail.com wrote: > Hi kib@! >=20 > On Sep 9, 2014, at 1:33, Konstantin Belousov <kostikbel@gmail.com> wrote: >=20 > > 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> wr= ote: > >>>=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 wou= ld > >>>> 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 bel= ow > >>>> for more details). I've attached a patch which addresses all hardcod= ed > >>>> 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 in= cluded" > >>>> $ gcc -D_KERNEL -c cdefs_pound_define.c > >>>> cdefs_pound_define.c:4:2: warning: #warning "sys/cdefs.h has been in= cluded? > >>>=20 > >>> I wouldn?t bother changing the nitems #define. There?s no need, reall= y, to do that. > >>=20 > >> Rethinking my proposal, I agree. I had lofty hopes for unifying the ma= cros, 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 d= oesn?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. >=20 > It?s a bit less painful if you omit cddl, crypto, and gnu: >=20 > $ git grep -e 'sizeof.*/sizeof' -- | egrep -v '^(cddl|contrib|crypto|gnu)= ' | grep '#.*define' | wc -l > 105 >=20 > > Not all of them are for equiv macros, but significant part repeats ntim= es() > > in variations. At least Linuxish ARRAY_SIZE/DRM_ARRAY_SIZE is much more > > popular than __arraysize() which appears three (!) times. >=20 > Hmm.. that?s a bit surprising. >=20 > > I do not see any reason to pollute central header cdefs.h with sparcely > > used macro for some compat code. >=20 > This is one compelling reason ? it?s used 225 times in the test code in N= etBSD, spanning 92 files, and 15 times spanning 10 files in the rump kernel. >=20 > $ grep -rl __arraycount netbsd/src/tests/ | wc -l; grep -r __arraycount n= etbsd/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 >=20 > My goal is to pull in as much test code from NetBSD that I?ve 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. >=20 > That being said, there might be a compromise that could be struck where t= he compat code can stay confined to libnetbsd. If I change the code bsd.tes= t.mk or contributed code to explicitly use -include or -I, then point to th= e lib/libnetbsd/sys/cdefs.h header, then this could become a non-issue: >=20 > -include filename > Adds an implicit #include into the predefines buffer which is = read > before the source file is preprocessed. >=20 > -Idirectory > Add the specified directory to the search path for include fil= es. >=20 > Does that seem like an appropriate solution? If it is isolated to tests, I have no objections nor comments. >=20 > Thanks! > -Garrett >=20 > PS For what it?s worth, nitems use has caught on like wildfire (it beats = out __arraycount and the ad hoc sizeof): >=20 > $ git grep -e nitems -- | egrep -v '^(cddl|contrib|crypto|gnu)' | wc -l > 551 It might be worth to clean up the __arraycount or other equivalent macros in the non-contributed code (only because the issue is already discussed much more than it worth). E.g., sys/kern/stack_protector.c can happily use nitems(), I am sure that there are much more such places. --WEBrq8xPpGVQldHe Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v2 iQIcBAEBAgAGBQJUD2sgAAoJEJDCuSvBvK1BZaEP/3bYiHQmUOOCUA3O3w0EllCm dJPm7PSfnbd0oAeFm6Y47dtH/napTcbctYGKe+/Dznd2wUz8sgFVhjXynlxBEazl LShyizXEel3c0et5KCZ9bkP58oio9Cd20jhb82KcX8hvvlN97UcuvHr59M9Zpxu6 Vs/p5WCNCv70uTcPn06R0GFO8ldgy2pTPSj9AvavfcNk++rb/rpvkl/Gt2h+74S5 UjiNI7gNn+MACYcgtaDvAbsug2M6X4d0aaS5b0YWuwj7YCx8QG/+1N8R2KCn6SrF r5BmgQ/1ngw7pbazvZ5cdc5AQ+qfMX/EmBeBYwkCthR9+RAh0FNJyRTbXB36D820 1X5nQw9buFZZNrFrAriiSkLQN4lYg4csHgQkErA88pcG8U7yNTnAScqQmFK7iFRK xfm5HeAsUw6qr2naCRoMnz75n0WODP5+TmcT1Np57/4nw5RpYFCexQNP1D0Wd7N4 XK8+OwrtBGgM52aVgJF9Dle9oxJntRtq6bdZyYCq613tS7OmBMd9lavCnWGht6cR BHwMTMs/Y7ijASa5whxHTCTtGQ5FIIVKqNlQT5d1YgzrXiNLARDDu9Z6UYK/UjiZ pbRmyIhdi3o/J5qgLW9pOcI4O6FcDwe0NxRaN+GqjxjSgRUQlGX5D5xAIv0kezuH DkUa3IeS11cnR/wHmKD4 =fmQy -----END PGP SIGNATURE----- --WEBrq8xPpGVQldHe--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20140909210329.GE2737>