On 10/04/2019 07:08, Mike Kaganski wrote:
On 10.04.2019 2:11, Thorsten Behrens wrote:
So this is apparently about <https://gerrit.libreoffice.org/plugins/gitiles/core/+/d38f9934f08939032cca64a32de58fa3901a88d5%5E!/> "[API CHANGE] Asserts to never clear already cleared guard".
--- a/include/osl/mutex.hxx +++ b/include/osl/mutex.hxx @@ -178,11 +178,9 @@ namespace osl */ void clear() { - if(pT) - { - pT->release(); - pT = NULL; - } + assert(pT); + pT->release(); + pT = NULL; } };This will have unsuspecting consumers of our API crash if they don't catch the assertion during development. I'm not sure that's a positive thing to impose on our ecosystem (where LibreOffice support might already not be a priority). I'd be much happier with the pT check still present, but guarded by !LIBO_INTERNAL_ONLY.Please check the discussion at https://gerrit.libreoffice.org/70381: in fact, my original intention was just that, and it was implemented to only assert in debug builds.
Note that there is a difference between "defensive programming" as in <https://gerrit.libreoffice.org/#/c/70381/2/include/osl/mutex.hxx> vs. narrowing the interface of ClearableGuard::clear only for LIBO_INTERNAL_ONLY.
(And if there is concerns that we should do the latter, I am fine with that.)