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
_______________________________________________
svn-src-all@freebsd.org mailing list
http://lists.freebsd.org/mailman/listinfo/svn-src-all
To unsubscribe, send any mail to "svn-src-all-unsubscr...@freebsd.org"

Reply via email to