svn commit: r296868 - head/sys/sys

Bruce Evans brde at optusnet.com.au
Tue Mar 15 09:23:26 UTC 2016


On Tue, 15 Mar 2016, Hans Petter Selasky wrote:

> On 03/14/16 21:33, Gleb Smirnoff wrote:
>> On Mon, Mar 14, 2016 at 02:31:37PM -0400, Ryan Stone wrote:
>> R> On Mon, Mar 14, 2016 at 2:07 PM, Gleb Smirnoff <glebius at freebsd.org> 
>> wrote:
>> R>
>> R> >   Remove useless cast in SYSCTL_ADD_COUNTER_U64 macro.

There was no cast here.

>> R> Is it useless?  I believe that the point is to give a compiler error if 
>> an
>> R> incompatible pointer type was passed as the ptr parameter.
>> 
>> Thanks for explanation! I will back out.
>
> I added the casts to get more checks with regard to the type safety in the 
> sysctls, because sysctl_add_oid() uses "void *". They are not useless.

Casts would reduce type safety.  You actually added assignments.
Assignments to void * would be little different from conversions to the
void * parameter, but the assignments are to uint64_t * so they detect
most type differences except an initial type of void * which might be
correct (the caller should have started with uint64_t *, and it is too
late to detect the error if the caller started with something like
const volatile int32_t * and subverted this to void *.  But if the
sysctl is read-only, subverting its pointer from const int64_t * to
int64_t * might be needed since lower levels don't support const).

> Maybe this should be explained in a comment somewhere?

A comment is not a place to explain what a cast is.

The assignment doesn't work for what should the usual case of a static
initializer.  I don't see a good way to fix this.  A not so good way is
to add a union holding int *, long *, etc. to the sysctl data.  Then for
SYSCTL_INT() assign the pointer to the the int * union member, etc.

Bruce


More information about the svn-src-head mailing list