On Tue, Jan 15, 2019 at 03:05:50PM +0100, Johannes Schindelin wrote: > > But certainly it would not be the first such instance in our code base. > > Just because a lot of our code has grown historically does not mean that > we need to add code that shares the same shortcomings. FREE_AND_NULL() was > not available for a long time, after all, so it is understandable that we > did not use it back then. But it is available now, so we no longer have an > excuse to add less defensive code. Fair enough. I am happy to start using FREE_AND_NULL() consistently if nobody things it's too opaque (or that it creates confusion that we somehow expect to look at the variable again and need it to be NULL). I think the compiler should generally be able to optimize out the NULL assignment in most cases anyway. > > In theory a static analyzer should easily be able to figure out such a > > problem, too, so maybe it is not worth being defensive about. > > How often do you run a static analyzer? Stefan was routinely running coverity, though I haven't seen results in a while. I think we should make sure that continues, as it did turn up some useful results (and a lot of cruft, too, but on the whole I have found it useful). I also count gcc as a static analyzer, since it can and does point out many simple problems. I don't know that it can point out an obvious user-after-free like this, though. That said.... > My point being: if we can prevent future mistakes easily, and it does not > add too much code churn, why not just do it. No need to rely on fancy > stuff that might not even be available on your preferred platform. Yeah, I do agree (which is why I included it in the patch ;) ). -Peff
