Date: Thu, 12 Oct 2006 11:20:36 +0400 From: Ruslan Ermilov <ru@FreeBSD.org> To: Bruce Evans <bde@zeta.org.au> Cc: freebsd-net@FreeBSD.org, andre@FreeBSD.org Subject: Re: [PATCH] Make hash.h usable in the kernel Message-ID: <20061012072036.GA60767@rambler-co.ru> In-Reply-To: <20061012052101.A814@epsplex.bde.org> References: <20061011090241.GA2831@FreeBSD.czest.pl> <20061011094049.GA24964@rambler-co.ru> <20061012052101.A814@epsplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
--XsQoSWH+UP9D9v3l
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
Content-Transfer-Encoding: quoted-printable
On Thu, Oct 12, 2006 at 05:21:09AM +1000, Bruce Evans wrote:
> On Wed, 11 Oct 2006, Ruslan Ermilov wrote:
> >%%%
> >Index: sys/sys/hash.h
> >=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
> >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 =3D buf;
> >
>=20
> I think this would break passing ep in almost all callers,
>=20
There are no callers of these functions yet, at least not in the
current FreeBSD kernel. There are only 2 callers in OpenBSD,
both in sys/kern/vfs_lookup.c
> in the same
> way that "fixing" the corresponding arg in the strtol(3) family would
> break almost all callers.
>=20
Yes, but strtol(3) has seen more life in sin. ;)
> Callers want and need to pass char **, but
> char ** is not compatible with const char **.
>=20
Not compatible, but "char **" can safely be casted to "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.
>=20
But this is bad practice; if string is really const, writing to *end
will SIGBUS, and the fact that interface has it spelled as "char **"
doesn't mitigate it:
: #include <stdlib.h>
:=20
: static const char *s =3D "123a";
:=20
: int
: main(void)
: {
: long v;
: char *endptr;
:=20
: endptr =3D NULL;
: v =3D strtol(s, &endptr, 0);
: if (endptr !=3D NULL)
: *endptr =3D '\0';
: return (0);
: }
OTOH, if string is really modifiable, then simple casting when calling
a function works:
: #include <stdlib.h>
:=20
: void foo(const char *, char *);
: void bar(const char *, const char **);
:=20
: void
: foo(const char *s1, char *s2)
: {
: const char *end1 =3D NULL;
: char *end2 =3D NULL;
:=20
: bar(s1, &end1);
: bar(s2, (const char **)&end2);
: }
Or differently: it's safe (and possible) to do "end1 =3D end2",
but not the opposite.
> Changing
> the prototype forces all callers to use "const char **end; ... &end",
^ extra `*'
> and then if they want to modify *end, to convert `end' to plain char *.
>=20
Not necessarily, see above. And from the function's POV (whose prototype
we're considering), "end" will be made to point to a substring of a const
string, so obviously it will also point to a const string.
> 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.
>=20
Not *lots* actually. Passing "char *" where "const char *" is required
is safe and allowed, passing "char **" as "const char **" is allowed but
requires a (safe) cast.
> >@@ -94,7 +94,7 @@ hash32_stre(const void *buf, int end, ch
> > hash =3D HASHSTEP(hash, *p++);
> >
> > if (ep)
> >- *ep =3D (char *)p;
> >+ *ep =3D (const char *)p;
> >
> > return hash;
> >}
>=20
> Doesn't this cause a cast-qual warning in the kernel?
>=20
Why? None of qualifiers are lost as a result of cast; both "p" and "ep"
are pointers to const-qualified base types. (No, it doesn't cause a
warning.)
Cheers,
--=20
Ruslan Ermilov
ru@FreeBSD.org
FreeBSD committer
--XsQoSWH+UP9D9v3l
Content-Type: application/pgp-signature
Content-Disposition: inline
-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.5 (FreeBSD)
iD8DBQFFLezEqRfpzJluFF4RArz+AJwPTP7EDDuEcZDi4aF49yIw8CBoLgCdEFLI
+uGMOqytUx8bpaxFFqX/3xw=
=r2jR
-----END PGP SIGNATURE-----
--XsQoSWH+UP9D9v3l--
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20061012072036.GA60767>