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.)
_______________________________________________
LibreOffice mailing list
LibreOffice@xxxxxxxxxxxxxxxxxxxxx
https://lists.freedesktop.org/mailman/listinfo/libreoffice
