From owner-freebsd-net@FreeBSD.ORG Wed Oct 11 19:21:24 2006 Return-Path: X-Original-To: freebsd-net@FreeBSD.org Delivered-To: freebsd-net@FreeBSD.org Received: from mx1.FreeBSD.org (mx1.freebsd.org [216.136.204.125]) by hub.freebsd.org (Postfix) with ESMTP id 98FA716A407; Wed, 11 Oct 2006 19:21:24 +0000 (UTC) (envelope-from bde@zeta.org.au) Received: from mailout1.pacific.net.au (mailout1-3.pacific.net.au [61.8.2.210]) by mx1.FreeBSD.org (Postfix) with ESMTP id D425D43D5E; Wed, 11 Oct 2006 19:21:12 +0000 (GMT) (envelope-from bde@zeta.org.au) Received: from mailproxy1.pacific.net.au (mailproxy1.pacific.net.au [61.8.2.162]) by mailout1.pacific.net.au (Postfix) with ESMTP id F3F775A0E70; Thu, 12 Oct 2006 05:21:10 +1000 (EST) Received: from epsplex.bde.org (katana.zip.com.au [61.8.7.246]) by mailproxy1.pacific.net.au (Postfix) with ESMTP id D41F98C0C; Thu, 12 Oct 2006 05:21:09 +1000 (EST) Date: Thu, 12 Oct 2006 05:21:09 +1000 (EST) From: Bruce Evans X-X-Sender: bde@epsplex.bde.org To: Ruslan Ermilov In-Reply-To: <20061011094049.GA24964@rambler-co.ru> Message-ID: <20061012052101.A814@epsplex.bde.org> References: <20061011090241.GA2831@FreeBSD.czest.pl> <20061011094049.GA24964@rambler-co.ru> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: freebsd-net@FreeBSD.org, andre@FreeBSD.org Subject: Re: [PATCH] Make hash.h usable in the kernel X-BeenThere: freebsd-net@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: Networking and TCP/IP with FreeBSD List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Oct 2006 19:21:24 -0000 On Wed, 11 Oct 2006, Ruslan Ermilov wrote: > On Wed, Oct 11, 2006 at 11:02:41AM +0200, Wojciech A. Koszek wrote: >> I'm working on potential consumer of functions from sys/hash.h. Currently, I >> can't make them work without modyfication in my sample KLD. This is a patch >> which fixes the problem: >> ... > This is a wrong fix. A correct fix would be: > > %%% > Index: sys/sys/hash.h > =================================================================== > RCS file: /home/ncvs/src/sys/sys/hash.h,v > retrieving revision 1.2 > diff -u -p -r1.2 hash.h > --- sys/sys/hash.h 12 Mar 2006 15:34:33 -0000 1.2 > +++ sys/sys/hash.h 11 Oct 2006 09:38:50 -0000 > @@ -86,7 +86,7 @@ hash32_strn(const void *buf, size_t len, > * namei() hashing of path name parts. > */ > static __inline uint32_t > -hash32_stre(const void *buf, int end, char **ep, uint32_t hash) > +hash32_stre(const void *buf, int end, const char **ep, uint32_t hash) > { > const unsigned char *p = buf; > I think this would break passing ep in almost all callers, in the same way that "fixing" the corresponding arg in the strtol(3) family would break almost all callers. Callers want and need to pass char **, but char ** is not compatible with const char **. Callers want to do this because it's easier to write "char *end; ... &end", and they often need to do this so that they can modify the resulting *end. Changing the prototype forces all callers to use "const char **end; ... &end", and then if they want to modify *end, to convert `end' to plain char *. Modifying *end is only valid if the original string is modifyable, and this case ends up needing lots of ugly casting away of const, which leads to compiler warnings, which lead to even uglier things like the __DECONST() mistake to "fix" the warnings. Similarly for the strchr(3) family. > @@ -94,7 +94,7 @@ hash32_stre(const void *buf, int end, ch > hash = HASHSTEP(hash, *p++); > > if (ep) > - *ep = (char *)p; > + *ep = (const char *)p; > > return hash; > } Doesn't this cause a cast-qual warning in the kernel? > @@ -105,7 +105,7 @@ hash32_stre(const void *buf, int end, ch > * as a helper for the namei() hashing of path name parts. > */ > static __inline uint32_t > -hash32_strne(const void *buf, size_t len, int end, char **ep, uint32_t hash) > +hash32_strne(const void *buf, size_t len, int end, const char **ep, uint32_t hash) Line too long. Bruce