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
> >===================================================================
> >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,
>
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.
>
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 **.
>
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.
>
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>
:
: static const char *s = "123a";
:
: int
: main(void)
: {
: long v;
: char *endptr;
:
: endptr = NULL;
: v = strtol(s, &endptr, 0);
: if (endptr != NULL)
: *endptr = '\0';
: return (0);
: }
OTOH, if string is really modifiable, then simple casting when calling
a function works:
: #include <stdlib.h>
:
: void foo(const char *, char *);
: void bar(const char *, const char **);
:
: void
: foo(const char *s1, char *s2)
: {
: const char *end1 = NULL;
: char *end2 = NULL;
:
: bar(s1, &end1);
: bar(s2, (const char **)&end2);
: }
Or differently: it's safe (and possible) to do "end1 = 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 *.
>
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.
>
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 = 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?
>
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,
--
Ruslan Ermilov
[EMAIL PROTECTED]
FreeBSD committer
pgpuLayyf2lx0.pgp
Description: PGP signature
