Date: Wed, 16 Jun 2010 00:06:50 +1000 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Andriy Gapon <avg@freebsd.org> Cc: Kostik Belousov <kostikbel@gmail.com>, svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r209193 - head/sys/dev/sound/pcm Message-ID: <20100615231528.A38864@delplex.bde.org> In-Reply-To: <4C173575.1080003@freebsd.org> References: <201006150706.o5F76sLF004481@svn.freebsd.org> <20100615080309.GK13238@deviant.kiev.zoral.com.ua> <4C173575.1080003@freebsd.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 15 Jun 2010, Andriy Gapon wrote: > on 15/06/2010 11:03 Kostik Belousov said the following: >> On Tue, Jun 15, 2010 at 07:06:54AM +0000, Andriy Gapon wrote: >>> Log: >>> sound/pcm: use non-const string as a value with SYSCTL_STRING >>> >>> Although the sysctls are marked with CTLFLAG_RD and the values will stay >>> immutable, current sysctl implementation stores value pointer in >>> void* type, which means that const qualifier is discarded anyway >>> and some newer compilers complaint about that. >>> We can't use de-const trick in sysctl implementation, because in that >>> case we could miss an opposite situation where a const value is used >>> with CTLFLAG_RW sysctl. __DECONST() and __DEVOLATILE can almost never be used, because they just hide bugs like the above or API bugs in most cases, and I would complain about their use in all cases. >>> >>> Complaint from: gcc 4.4, clang >>> MFC after: 2 weeks >> This is arguably the change for worse then better. > > Arguably - yes, practically - I am not sure. > See almost every other instance of SYSCTL_STRING usage, kern_mib.c most prominently. > >> You could add SYSCTL_STRING_CONST or the like instead. > > But we already have CTLFLAG_RD vs CTLFLAG_RW... > Perhaps, we could have a union of void* and const void* and then assign value to > the appropriate member based on the flags. But I am not sure if the benefit > would be worth the effort. More importantly the "void *" causes many type mismatches to be missed. E.g., SYSCTL_INT() should only accept variables of type int, but the variable type decays to "void *" almost before it can be checked, and SYSCTL_INT() depends on this decay (for the assignment). I couldn't see a good way to fix this. A bad way might be to use an enormous union with all supported types and type qualifiers. SYSCTL_INT() can then assign to the unqualified int type. But this only fixes the easier cases where the macro is used. Many cases use a SYSCTL_PROC() with a sysctl_handle_foo() call. All these calls take args of SYSCTL_HANDLER_ARGS. This gives the same types for all the calls and one of the types is "void *" which defeats type safety in the same way as the assignment to the "void *" in the macros. We now have flags for some unsigned types (CTLTYPE_UQUAD is still missing), but we don't have sysctl_handle_uint() and depend on this decay for for sysctl_handle_int() to work on u_ints. But this type punning is a feature. At least the macro version can do a static type check by doing an assignment without a conversion, and then assign to a "void *" or a slightly more specialized type as now, to avoid replicating the handler functions. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20100615231528.A38864>