<div dir="ltr"><br><br>On Tue, Jan 20, 2015 at 9:10 AM, Stephan Bergmann <<a href="mailto:sbergman@redhat.com">sbergman@redhat.com</a>> wrote:<br>> [...]<br>>><br>>> I know some significant work has already been done in cleaning up<br>>> assertions. This in no way invalidates them at all.<br>>> In fact, changing assert to LO_ASSERT is trivial. The effort to<br>>> convert the OSL_ and DBG_ ones remains the same (with or without the<br>>> above proposal).<br>>><br>>> However, LO_ASSERT will support the above advantages and give full<br>>> control over assertions in the project. The cost is very low (surely<br>>> searching and replacing assert with LO_ASSERT is not a high cost). The<br>>> variants will allow more fine-grained control and will make the<br>>> behavior more homogeneous.<br>>><br>>> I hope you agree that this is an improvement over, and not a<br>>> competition with, the current effort to replace OSL_ and DBG_ versions<br>>> with assert.<br>><br>><br>> Over the years, we've seen lots of attempts come and go to devise<br>> sophisticated assertion/logging/debugging mechanisms, none of which lead to<br>> a clean, consistent code base.  The current approach (assert, SAL_WARN,<br>> SAL_INFO) is deliberately simplistic.<br>><br>> I would not say that you are not raising relevant points, but at this point<br>> I would rather like to see fdo#43157 getting fixed, and not introduce yet<br>> another approach into the zoo.<br><br>I hear you, and understand the general reluctance.<br>I've made every effort to make the code as simple as possible (please do checkout patch 13945).<br>At its core, I'm suggesting to wrap around assert() with something we can control and is more powerful.<br>The other two versions (one always enabled and another that is enabled only when OSL_DEBUG_LEVEL > 1) can be postponed.<br><br>That is, I'm suggesting using the proposed LO_ASSERT instead of assert() and _optionally_ add a custom message and/or dump variables. At the very least it will be an in-place replacement that gives us more control than the system default. Initially, and by default, LO_ASSERT could log and invoke assert(). Again, this wouldn't compete or distract from fdo#43157, it'll only make it more fruitful.<br><br>I'd like to hear comments as to how to improve the patch to hone it into a form that would get merged eventually. I'm willing to simplify if there is concern that it's over-engineered. However I think <font face="monospace, monospace">LO_ASSERT(index < size, "No pigeon at </font><span style="font-family:monospace,monospace">(%d), only (%d)</span><font face="monospace, monospace"> holes.", index, size);</font> is much more powerful than <font face="monospace, monospace">assert(index < size);</font> Although <font face="monospace, monospace">LO_ASSERT(index < size);</font> is also possible and isn't any less readable.</div>