From owner-svn-src-all@FreeBSD.ORG Tue Jun 15 14:06:56 2010 Return-Path: Delivered-To: svn-src-all@freebsd.org Received: from mx1.freebsd.org (mx1.freebsd.org [IPv6:2001:4f8:fff6::34]) by hub.freebsd.org (Postfix) with ESMTP id 2C13C1065670; Tue, 15 Jun 2010 14:06:56 +0000 (UTC) (envelope-from brde@optusnet.com.au) Received: from mail03.syd.optusnet.com.au (mail03.syd.optusnet.com.au [211.29.132.184]) by mx1.freebsd.org (Postfix) with ESMTP id 8BE3C8FC17; Tue, 15 Jun 2010 14:06:55 +0000 (UTC) Received: from c122-106-175-69.carlnfd1.nsw.optusnet.com.au (c122-106-175-69.carlnfd1.nsw.optusnet.com.au [122.106.175.69]) by mail03.syd.optusnet.com.au (8.13.1/8.13.1) with ESMTP id o5FE6pYl014814 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES256-SHA bits=256 verify=NO); Wed, 16 Jun 2010 00:06:52 +1000 Date: Wed, 16 Jun 2010 00:06:50 +1000 (EST) From: Bruce Evans X-X-Sender: bde@delplex.bde.org To: Andriy Gapon In-Reply-To: <4C173575.1080003@freebsd.org> Message-ID: <20100615231528.A38864@delplex.bde.org> References: <201006150706.o5F76sLF004481@svn.freebsd.org> <20100615080309.GK13238@deviant.kiev.zoral.com.ua> <4C173575.1080003@freebsd.org> MIME-Version: 1.0 Content-Type: TEXT/PLAIN; charset=US-ASCII; format=flowed Cc: Kostik Belousov , svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org Subject: Re: svn commit: r209193 - head/sys/dev/sound/pcm X-BeenThere: svn-src-all@freebsd.org X-Mailman-Version: 2.1.5 Precedence: list List-Id: "SVN commit messages for the entire src tree \(except for " user" and " projects" \)" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Tue, 15 Jun 2010 14:06:56 -0000 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