Date: Wed, 12 Nov 2014 11:20:59 +1100 (EST) From: Bruce Evans <brde@optusnet.com.au> To: Brooks Davis <brooks@freebsd.org> Cc: src-committers@freebsd.org, svn-src-all@freebsd.org, "Bjoern A. Zeeb" <bz@freebsd.org>, Bruce Evans <brde@optusnet.com.au>, svn-src-head@freebsd.org, Dag-Erling Sm?rgrav <des@des.no> Subject: Re: svn commit: r274340 - in head/sys: crypto/rijndael dev/random geom/bde Message-ID: <20141112105202.F1068@besplex.bde.org> In-Reply-To: <20141111163105.GA69731@spindle.one-eyed-alien.net> References: <201411100944.sAA9icnN061962@svn.freebsd.org> <3C962D07-3AAF-42EA-9D3E-D8F6D9A812B0@FreeBSD.org> <86sihq5a2v.fsf@nine.des.no> <20141111223756.F3519@besplex.bde.org> <86oasd6dad.fsf@nine.des.no> <20141111163105.GA69731@spindle.one-eyed-alien.net>
next in thread | previous in thread | raw e-mail | index | archive | help
On Tue, 11 Nov 2014, Brooks Davis wrote: > On Tue, Nov 11, 2014 at 03:07:54PM +0100, Dag-Erling Sm?rgrav wrote: >> I agree that __DECONST() is ugly (not least because it strips all >> qualifiers, not just const, so it should be DEQUAL()), but the >> alternative is worse. In my experience, the majority of cases where a >> cast discards a qualifier are bugs, with struct iov being one of very >> few legitimate use cases. > > On the processor we (SRI and Cambridge) are working on, pointers > are not integers (we support some integer behaviors, but not > pointer->int->pointer casts except in limited cases) and the current > __DECONST implementation will need to die[0]. If uintptr_t doesn't work, then __DECONST() is the least of your problems. Kernel code, especially, has lots of assumptions that uintptr_t can represent pointers. Also that vm_offset_t can represent addresses, and probably that vm_offset_t is exactly the same as uintptr_t. Once you have integer that can represent pointers (which is always possible by implementing integers with enough bits), it is easily for the compiler to arrange that casting through it works to give the semantics of [u]intptr_t. Since I rmrf'ed __DECONST() locally, I didn't fix bugs in it like casting back to (type) without going through (void *). uintptr_t is only specified to work for void *. Once the result is void *, there is no need for the 'type' arg in the API. A void * result can be implicitly in C or explicitly in C++ be converted to any other pointer type. Another bug in __DECONST() is that its implementation depends on the bug that -Wcast-qual is broken for casting to uintptr_t. Casting to uintptr_t certainly removes the type qualify. The only correct implementation to break the compiler warning seems to be to memcpy() the pointer to an array of bits, then memcpy() the bits back to a pointer. On most arches, 'const' is virtual and not encoded in the bits, so this gives the correct result in an honestly unportable way. > For existing C versions > some sort of compiler support for __DECONST is probably the right thing > to do. In general, we need to fix the C/C++ standard to us express the > things we actually mean when we use const (for example see strchr()'s > use of const). I believe the last issue now being tracked on Google's > internal list of deficiencies in the C++ standard. As I said, the compiler support is the C cast operator. We just break the cast operator since we know that casts are often wrong and we want warnings about some cases. The nicest syntax for getting back the cast operator is repeating casts 3 times to say that you really mean them :-). > [0] The recently discussed _ALIGN also needs to die and be replaced with > something that increments the pointer (or returns how much to increment) > rather than jamming it though a long. _ALIGN() needs honest unportability since it changes the bits. I don't see how the compiler can help significantly. The pointer that you start with is usually obtained by illegitimate means. Typically from an offset in an array of bytes, where the array isn't even a C array but is a hardware memory region. It is likely to be just as easy to hack on this offset to align it than to construct a C pointer to the start of the memory region. Bruce
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?20141112105202.F1068>