On 6/22/19 17:15, Conrad Meyer wrote: > Hi Xin Li, > > On Mon, Jun 17, 2019 at 12:49 PM Xin LI <delp...@freebsd.org> wrote: >> >> Author: delphij >> Date: Mon Jun 17 19:49:08 2019 >> New Revision: 349151 >> URL: https://svnweb.freebsd.org/changeset/base/349151 >> >> Log: >> Separate kernel crc32() implementation to its own header (gsb_crc32.h) and >> rename the source to gsb_crc32.c. >> >> This is a prerequisite of unifying kernel zlib instances. >> >> ... >> Modified: head/sys/libkern/x86/crc32_sse42.c >> ============================================================================== >> --- head/sys/libkern/x86/crc32_sse42.c Mon Jun 17 17:35:55 2019 >> (r349150) >> +++ head/sys/libkern/x86/crc32_sse42.c Mon Jun 17 19:49:08 2019 >> (r349151) >> @@ -29,14 +29,14 @@ __FBSDID("$FreeBSD$"); >> /* >> * This file is compiled in userspace in order to run ATF unit tests. >> */ >> -#ifdef USERSPACE_TESTING >> +#ifndef _KERNEL > > This change and following identical changes revert a request by kib@ > in https://reviews.freebsd.org/D9342 . (When this revision was > initially proposed in , it was '#ifndef _KERNEL' — kib@ requested the > use of a different preprocessor macro.)
Thanks for pointing this out -- admittedly, I haven't read the reasoning before. But after reading more about the original review context, I don't quite agree with Kostik's reasoning because the code actually works in userland regardless of the original intention. It's true that the code is at the time only used in the test program, but they do enabled usage in userland, and they by themselves do not serve purely test purposed task (these are not mock interfaces, nor test cases, etc.), and the common practice in sys/libkern is to wrap these with #if[n]def _KERNEL. I don't insist but I still think it's more reasonable to use _KERNEL because it's more consistent with the surrounding code. Cheers,
signature.asc
Description: OpenPGP digital signature
