From owner-svn-src-all@freebsd.org Sun Jun 23 11:23:23 2019 Return-Path: Delivered-To: svn-src-all@mailman.ysv.freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2610:1c1:1:606c::19:1]) by mailman.ysv.freebsd.org (Postfix) with ESMTP id 7514515CDFB9; Sun, 23 Jun 2019 11:23:23 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from kib.kiev.ua (kib.kiev.ua [IPv6:2001:470:d5e7:1::1]) (using TLSv1.3 with cipher TLS_AES_256_GCM_SHA384 (256/256 bits) server-signature RSA-PSS (4096 bits)) (Client did not present a certificate) by mx1.freebsd.org (Postfix) with ESMTPS id A48A486CF4; Sun, 23 Jun 2019 11:23:22 +0000 (UTC) (envelope-from kostikbel@gmail.com) Received: from tom.home (kib@localhost [127.0.0.1]) by kib.kiev.ua (8.15.2/8.15.2) with ESMTPS id x5NBNFPr087905 (version=TLSv1.3 cipher=TLS_AES_256_GCM_SHA384 bits=256 verify=NO); Sun, 23 Jun 2019 14:23:18 +0300 (EEST) (envelope-from kostikbel@gmail.com) DKIM-Filter: OpenDKIM Filter v2.10.3 kib.kiev.ua x5NBNFPr087905 Received: (from kostik@localhost) by tom.home (8.15.2/8.15.2/Submit) id x5NBNF45087900; Sun, 23 Jun 2019 14:23:15 +0300 (EEST) (envelope-from kostikbel@gmail.com) X-Authentication-Warning: tom.home: kostik set sender to kostikbel@gmail.com using -f Date: Sun, 23 Jun 2019 14:23:15 +0300 From: Konstantin Belousov To: Xin Li Cc: cem@freebsd.org, src-committers , svn-src-all , svn-src-head , kib@freebsd.org Subject: Re: svn commit: r349151 - in head: lib/libufs stand/libsa sys/conf sys/dev/iscsi sys/dev/iscsi_initiator sys/dev/liquidio sys/dev/usb/net sys/fs/ext2fs sys/fs/nandfs sys/geom/part sys/geom/raid sys/ker... Message-ID: <20190623112315.GS8697@kib.kiev.ua> References: <201906171949.x5HJn9Ed091108@repo.freebsd.org> <58af4bc5-0097-1540-be64-e9bceab6d4f8@FreeBSD.org> MIME-Version: 1.0 Content-Type: text/plain; charset=utf-8 Content-Disposition: inline Content-Transfer-Encoding: 8bit In-Reply-To: <58af4bc5-0097-1540-be64-e9bceab6d4f8@FreeBSD.org> User-Agent: Mutt/1.12.0 (2019-05-25) X-Spam-Status: No, score=-1.0 required=5.0 tests=ALL_TRUSTED,BAYES_00, DKIM_ADSP_CUSTOM_MED,FORGED_GMAIL_RCVD,FREEMAIL_FROM, NML_ADSP_CUSTOM_MED autolearn=no autolearn_force=no version=3.4.2 X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on tom.home X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Sun, 23 Jun 2019 11:23:23 -0000 On Sat, Jun 22, 2019 at 09:13:05PM -0700, Xin Li wrote: > On 6/22/19 17:15, Conrad Meyer wrote: > > Hi Xin Li, > > > > On Mon, Jun 17, 2019 at 12:49 PM Xin LI 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. #ifndef _KERNEL is used when you provide generally useful code which must be limited to userspace. There, the conditionally compiled code is very specific, it is only for testing. Being explicit would allow to avoid questions, e.g. why userspace requires constructors. And still see my confusion in the referenced review.