Date: Tue, 15 Mar 2016 20:04:22 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Hans Petter Selasky <hps@selasky.org> Cc: Gleb Smirnoff <glebius@freebsd.org>, Ryan Stone <rysto32@gmail.com>, "src-committers@freebsd.org" <src-committers@freebsd.org>, "svn-src-all@freebsd.org" <svn-src-all@freebsd.org>, "svn-src-head@freebsd.org" <svn-src-head@freebsd.org> Subject: Re: svn commit: r296868 - head/sys/sys Message-ID: <20160315192238.B1463@besplex.bde.org> In-Reply-To: <56E7BD28.2000606@selasky.org> References: <201603141807.u2EI7xqm075602@repo.freebsd.org> <CAFMmRNyDuFGa=PYSgkfHpVsLxdKrFetpxrOmeFEf=YGVB1MAjQ@mail.gmail.com> <20160314203318.GL1328@FreeBSD.org> <56E7BD28.2000606@selasky.org>
next in thread | previous in thread | raw e-mail | index | archive | help
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@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
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20160315192238.B1463>