On 6 September 2011 21:52, Paul Pluzhnikov wrote: > On Tue, Sep 6, 2011 at 1:33 PM, Jonathan Wakely <[email protected]> wrote: > >>> for (it = v->begin(); it != v->end(); ++it) // Oops! >> >> Eurgh, the occurrence of "delete" in anything except a destructor is a >> code smell that should have led someone to find those bugs anyway! >> Obviously the code above is a trivial example, but it's unforgivable >> to write that > > I can assure you that the actual code that exhibited these bugs was much > subtler than that, and the bugs were not immediately obvious. > > Sometimes they were not immediately obvious even after running Valgrind > on the buggy code and getting allocation/deletion/access stack traces with > source corrdinates. > >>> We can't (easily) catch the general problem. This patch allows us to easily >>> catch this particular instance of it. >> >> Sure, but so does adding "assert(this);" at the top of every member > > Sorry, no. Adding "assert(this);" does not catch any new bugs (at least > not on Linux) -- the code would have immediately crashed on zero-page > dereference anyway.
I don't mean for vector::begin and the other functions in that patch, I mean in general for member functions of any type. There are plenty of functions that wouldn't crash when called through a null pointer. But even std:vector has member functions like that, such as max_size. Anyway, I think we've concluded the patch is not suitable for general use, as it has limited value without a debugging allocator that makes pages dirty after free.
