Date: Sat, 15 Jan 2011 08:25:03 -0800 From: mdf@FreeBSD.org To: Bruce Evans <brde@optusnet.com.au> Cc: svn-src-head@freebsd.org, svn-src-all@freebsd.org, src-committers@freebsd.org, Garrett Cooper <gcooper@freebsd.org> Subject: Re: svn commit: r217369 - in head/sys: cam/scsi sys Message-ID: <AANLkTimLgRijO6yeKd%2Bs2PokGDixL6oMJq_Zhdmco7-k@mail.gmail.com> In-Reply-To: <20110116013102.E21684@besplex.bde.org> References: <201101131820.p0DIKXip059402@svn.freebsd.org> <AANLkTinh619WaGgq=5fFxTvEX0JPir34k8xb%2Bs6oSH8Y@mail.gmail.com> <20110114174719.D28159@besplex.bde.org> <AANLkTikVwuSO3h8tKeYXCvC6zqYVHVxdY5Abrzo-Ks2R@mail.gmail.com> <20110115133929.D16210@besplex.bde.org> <AANLkTi=B5mZCJ_bhe=Gf1pLUmWsTswi3O3U2ZLnHMODV@mail.gmail.com> <20110115170529.T16715@besplex.bde.org> <AANLkTimoT9c6q2=K=C7dSnxJXkT=qZx=wPbAf6k68hWZ@mail.gmail.com> <20110116013102.E21684@besplex.bde.org>
next in thread | previous in thread | raw e-mail | index | archive | help
On Sat, Jan 15, 2011 at 6:55 AM, Bruce Evans <brde@optusnet.com.au> wrote: > On Sat, 15 Jan 2011, Garrett Cooper wrote: > >> On Fri, Jan 14, 2011 at 10:27 PM, Bruce Evans <brde@optusnet.com.au> >> wrote: >>> >>> On Fri, 14 Jan 2011, Garrett Cooper wrote: >>> >>>> On Fri, Jan 14, 2011 at 6:42 PM, Bruce Evans <brde@optusnet.com.au> >>>> wrote: > >>>>> ... >>>>> Oops. =A0I think sizeof() and issigned() can be used to determine the >>>>> type >>>>> well enough in functions and initialized data (do a fuller type check >>>>> if >>>>> the compiler supports it), but I don't know how to do this in static >>>>> sysctl declarations (since sizeof() can't be used in cpp expressions)= . >>>> >>>> =A0 Why not just create some dumb testcases that can be run at build >>>> time to determine that for you? >>> >>> Well, how? =A0You are given SYSCTL_I(&var, ...) and have to convert thi= s >>> to what is now in SYSCTL_INT(), using only the type of var, in hundreds >>> or thousands of files. =A0I don't even know how to do this with a test >>> ... >>> >>> =A0 =A0 =A0 =A0/* Only works for arithmetic types: */ >>> =A0 =A0 =A0 =A0#define isinteger(var) =A0((typeof(var))0.1 =3D=3D 0) >>> =A0 =A0 =A0 =A0#define issigned(var) =A0 ((typeof(var))-1 < 0) >>> =A0 =A0 =A0 =A0... >> >> =A0 This is what I meant: >> >> $ cat test_warnings.c >> #include <sys/types.h> >> >> size_t x =3D (int) -1; >> int y =3D 20000000000L; >> $ gcc -Wconversion -Wstrict-overflow -Wsign-compare -c test_warnings.c >> test_size_t.c:3: warning: negative integer implicitly converted to >> unsigned type >> test_size_t.c:4: warning: overflow in implicit constant conversion >> $ >> >> =A0 With the right CFLAGS and a few properly written tests, and a few >> make rules, you can figure out what's what pretty easily *shrugs*. > > That's a lot more parsing than seems reasonable. > > Anyway, we already depend on gnu __typeof() being avaible for the much > more central API of pcpu. =A0All pcpu accesses on amd64 and i386 use it, > except curthread (curthread has been hacked to use a more direct asm > for compile-time efficiency). > > SYSCTL_I() works even better that I first thought. =A0It automatically > gives support for all typedefed integral types. =A0No SYSCTL_FOO_T()s > with messy MD ifdefs for matching up foo_t with an integral type are > needed. =A0Instead there are even messier MI ifdefs in SYSCTL_I() :-). > But not so many. =A0There are hundreds of typedefed types of interest, > but only 8 different integer types to map to (8/16/32/64 bits signed > and unsigned). =A0The complication that int64_t is plain long on 64 > bit arches but long long on 32-bit arches still arises. =A0CTLTYPE_QUAD > can be associated with int64_t on all arches, but the format for printing > these is arch-dependent. =A0(Note that quad_t's cannot be printed using > %qd, and %qd is unusable, since %qd is just an alias for %lld, while > quad_t is an alias for int64_t and int64_t is plain long on 64-bit > arches so its format is %ld on these arches and %lld on others.) The printing is done entirely in user-space, so it's not too bad. I had figured to upcast everything to [u]intmax_t anyways, and what to cast from would just be [u]int32_t and [u]int64_t depending on reported size. Anything smaller should be copyout(9)'d as a 4 bit quantity. But, there's two factors at play here. The first is sysctl(8) which asks for the size when reading and manages it. The second is sysctl(2) where we have code like SCTL_MASK32 so a long on a 64-bit kernel looks like a long to a 32-bit app. It would be simpler to expose this as an 8-byte quantity on 64-bit kernel, and sysctl(8) will deal with this just fine, but that would break some uses of sysctl(2). However, I *think* it is safe to check the sysctl_req's oldidx/oldlen to see what size the user-space expects and cast a kernel long to int before SYSCTL_OUT. $WORK has gotten busy so I haven't had a chance to work on this much but I'm hopeful that the missus will watch the kids this weekend and allow me to hack. Thanks, matthew
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?AANLkTimLgRijO6yeKd%2Bs2PokGDixL6oMJq_Zhdmco7-k>