Re: CVS commit: src/sys

[Andrew Brown <atatat%atatdot.net@localhost>]

On Thu, Jun 09, 2005 at 01:07:34AM -0500, David Young wrote:
>On Thu, Jun 09, 2005 at 02:19:59AM +0000, Andrew Brown wrote:
>> 
>> Module Name: src
>> Committed By:        atatat
>> Date:                Thu Jun  9 02:19:59 UTC 2005
>> 
>> Modified Files:
>>      src/sys/kern: init_sysctl.c kern_sysctl.c uipc_domain.c vfs_bio.c
>>      src/sys/net: rtsock.c
>>      src/sys/net80211: ieee80211.c
>>      src/sys/netinet: ip_input.c tcp_usrreq.c
>>      src/sys/nfs: nfs_vfsops.c
>>      src/sys/sys: sysctl.h
>>      src/sys/ufs/lfs: lfs_vfsops.c
>> 
>> Log Message:
>> Properly fix the constipated lossage wrt -Wcast-qual and the sysctl
>> code.  I know it's not the prettiest code, but it seems to work rather
>> well in spite of itself.
>
>Andrew,
>
>It seems like the const sysctlnodes are const for a reason;

the nodes are const because only the core sysctl code should ever be
modifying it, not any of the helpers.  the "enforecement" of const by
the compiler provides a certain degree of protection from this.

>sysctl_lookup's argument is likewise non-const for a reason.  So why is
>it ever correct to pass an __UNCONST'd const sysctlnode to sysctl_lookup?

sysctl_lookup may modify the contents of a sysctlnode, so it's clearly
not const.

>This will not conceal bugs?

the application of const, while a nice ideal, inherently involves
bugs.  it either makes you give up on const altogether (doing nothing
for the bug situation that const may help with), cast things to get
around the const problems (thereby concealing bugs), or ends up making
the code overly complicated from dealing with const (so that new bugs
are introduced).

we could, as you suggested, add a SYSCTLFN_RWCALL (or however it was
to be named), but then almost all the current users (sysctl_lookup and
sysctl_query are >90%) of SYSCTLFN_CALL would have to be changed to
use the new macro.  that would probably lead most new code to use it,
too, since it would be the only thing in evident use, even though
you'd then be passing "rw" arguments to an "ro" function.  the
compiler doesn't care about that.

alternately, we could give up on all (well, most) use of const within
the sysctl code, thereby obviating the need for most (if not all) of
this debate.  of course, then we're all open to new code being written
that doesn't properly play by the rules.

or we could make all the sysctl functions more completely const, and
simply __UNCONST those arguments in those functions where it was
applicable.

personally, i think that where we are at the moment isn't all that
bad.  but...i'm open to change, if after reading this you still think
i'm wrong.

-- 
|-----< "CODE WARRIOR" >-----|
codewarrior%daemon.org@localhost             * "ah!  i see you have the internet
twofsonet%graffiti.com@localhost (Andrew Brown)                that goes 
*ping*!"
werdna%squooshy.com@localhost       * "information is power -- share the 
wealth."