[squid-dev] [PATCH] LockingPointer API update

[Christos Tsantilas]

On 06/22/2016 02:29 PM, Amos Jeffries wrote:
> On 22/06/2016 10:42 p.m., Christos Tsantilas wrote:
>> On 06/22/2016 07:32 AM, Amos Jeffries wrote:
>>> On 22/06/2016 1:02 p.m., Alex Rousskov wrote:
>>>> On 06/21/2016 04:00 AM, Amos Jeffries wrote:
>...
>
> In the patch I'm working on now its looking very much like we can drop
> the reset() method for these instances and just use a temporary local
> object. Like so:
>
> The above code becomes:
>
>    Security::CertPointer serverCert;
>    if (...) {
>      // We need to lock here because the destructor of
>      // serverCert will call X509_free and decrease the lock
>
>      // operator =() copy-assign does the lock stuff
>
>      serverCert = serverBump->serverCert;
>
>    } else {
>      // We do not need to lock here, the SSL_get_peer_certificate
>      // already increases the clock counter and we have to
>      // free with X509_free.
>
>      // CertPointer() constructor builds object without locking it
>      // assuming SSL_get_peer_certificate() did the lock for it.
>
>      // operator =() move-assign or copy-assign does the lock stuff
>
>      serverCert = Security::CertPointer(SSL_get_peer_certificate(ssl)));
>    }
>    // serverCert::~serverCert() called on function
>    // leave, calling X509_free.
>
>
> So long as we only construct LockingPointer from freshly minted OpenSSL
> raw-pointers that have an OpenSSL set lock. Then we do not need the
> non-locking reset() operation.
> We do need the locking reset*() method for the cases where the raw
> pointer is not locked by the library.

If we are going to implement and use "=" operator instead of reset 
method, then we should implement LockingPointer as a new full locking 
pointer, not a TidyPointer class.

I think this is will work.
The copy contructor and '=' operators should increase the lock, but a 
cunstructor which takes as argument the raw openSSL object (eg an "X509 
*") should not increase the lock.
However this is maybe is more confusing than the existing 
reset/resetAndLock scheme.

But I believe, this is will not solve your original problem, where you 
needed TidyPointer instead of LockingPointer.