On Tue, 10.12.13 18:33, Lennart Poettering ([email protected]) wrote: > On Tue, 10.12.13 05:24, Zbigniew Jędrzejewski-Szmek ([email protected]) wrote: > > > On Mon, Dec 09, 2013 at 09:22:43PM +0100, Thomas H.P. Andersen wrote: > > > If I understood correctly then the assert_return was meant to be used > > > only on the public library functions. I can't seem to find the > > > reference to it so maybe I am wrong though. > > > > There's no strong reason to limit usage to public functions. It's just > > a simple macro really. > > It's actually more complicated than that. The macro uses _unlikely_(), > which only should be used when we know that something is really really > unlikely. For assert()-style checks we know that thigns are unlikely, > since they are used to detect programming errors only, not runtime > errors. > > hence: please use assert_return() only for checks where normally an > assert() would be appropriate too, but where this would be too > unfriendly because it's in user-facing API. It's about being forgiving > to programming errors by users of our code, but not to ourselves. > > To underline this I have now updated assert_return() to also log the > failing condition as LOG_DEBUG, since that sounds like a good idea so > that programming errors can be easily tracked. > > So again: please do not replace if checks blindly by > assert_return(). Use assert_return() only to detect programming errors > in user code, which means only really to check parameters of public API > calls. Use assert() in internal code for the same purpose. > > I also updated CODING_STYLE a bit to explain this.
I have reverted this commit btw. This actually did blow up for example in bus_kernel_pop_memfd() which we call all the time on non-kdbus systems and which is supposed to fail there. Since assert_return() includes _likely_() using it here will hence slow things down quite a bit. Also, with my recent logging changes for assert_return() you actually get a lot of debug spew up this too, so it becomes extra-expensive. THe other changes in the same patch appeared to be in the same category: where it wasn't obvious that the checks would only detect programming errors and not happen in normal control flow. I have thus reverted the entire patch. Lennart -- Lennart Poettering, Red Hat _______________________________________________ systemd-devel mailing list [email protected] http://lists.freedesktop.org/mailman/listinfo/systemd-devel
