Date: Thu, 12 Oct 2006 05:21:09 +1000 (EST) From: Bruce Evans <bde@zeta.org.au> To: Ruslan Ermilov <ru@FreeBSD.org> Cc: freebsd-net@FreeBSD.org, andre@FreeBSD.org Subject: Re: [PATCH] Make hash.h usable in the kernel Message-ID: <20061012052101.A814@epsplex.bde.org> In-Reply-To: <20061011094049.GA24964@rambler-co.ru> References: <20061011090241.GA2831@FreeBSD.czest.pl> <20061011094049.GA24964@rambler-co.ru>
next in thread | previous in thread | raw e-mail | index | archive | help
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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061012052101.A814>