[Libreoffice-commits] core.git: forms/source framework/source include/osl

[Stephan Bergmann]

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.)