[squid-dev] [PATCH] TidyPointer removal

Amos Jeffries squid3 at treenet.co.nz
Wed Jun 29 11:45:12 UTC 2016


Attached patch completes everything except the LockingPointer parts, I
think.
Can it get some testing please while I continue on with that latter ?


On 29/06/2016 1:03 p.m., Alex Rousskov wrote:
> On 06/28/2016 08:52 AM, Amos Jeffries wrote:
>> On 28/06/2016 7:36 a.m., Alex Rousskov wrote:
>>> On 06/27/2016 04:35 AM, Amos Jeffries wrote:
>>>> This splits TidyPointer and LockingPointer by removing the inheritence
>>>> and copying the needed TidyPointer code into the LockingPointer as-is.
> 
> 
>>> Why start duplicating TidyPointer/unique_ptr functionality in
>>> LockingPointer?
> 
> 
>> So that LockingPointer is not affected by the unique_ptr change. 
> 
> That does not compute for me: Clearly, LockingPointer _is_ affected by
> the unique_ptr change in your patch. And if you meant that
> LockingPointer _callers_ should not be affected, then using unique_ptr
> for LockingPointer _implementation_ will not affect LockingPointer
> callers AFAICT.

Okay. How do we copy-construct a LockingPointer from another
LockingPointer with unique_ptr for values?
 unique_ptr requires they be unique, and states undefined behaviour if
any two are constructed from the same raw pointer value.

It only works for OpenSSL because the destructor does the unlocking.

> 
>> Without that LockingPointer would inherit from std::unique_ptr.
> 
> I hope you can avoid duplication without making unique_ptr a
> LockingPointer parent. You can use unique_ptr for the LockingPointer
> data member.
> 

Can you clarify please what you are referring to exactly when you use
the word "duplication" ?
which piece(s) of this patches LockingPointer code is duplicated ?


> 
>>> Can we [continue to] use unique_ptr in LockingPointer
>>> instead, now as a data member?
> 
>> I found we cannot. LockingPointer is doing too much non-unique_ptr
>> things with the locks. It is like a demented cross between unique_ptr,
>> shared_ptr and cbdata.
> 
> This does not compute for me: You have claimed that TidyPointer, the
> former LockingPointer parent, is essentially a unique_ptr. I agreed with
> that claim. Now you are claiming that you cannot use unique_ptr as a
> private data member of LockingPointer because LockingPointer does
> unusual things. Since you are not changing what LockingPointer does, I
> would expect LockingPointer to [continue to] be able to use
> TidyPointer/unique_ptr, regardless of how unusual LockingPointer is.
> Both claims cannot be true at the same time AFAICT...

And yet they are. That is the irrational situation I'm working to fix here.
TidyPointer took a raw* and did unique_ptr things to it.
LockingPointer took a raw* and sometimes did exclusively unique_ptr
things to it and sometimes used OpenSSL locks to make it shared /
ref-count then kept doing unique_ptr things to it (arg!).

Some caller code assumes its working with a unique_ptr (that logic doing
explicit move semantics I pointed out in the other thread). And some
caller code assumes its handling a shared pointer.

Dealing with that LockingPointer situation is still away in the future
for this patch. The previous and attached versions are just about
dropping TidyPointer into the abyss.

> 
> To make progress, I will rephrase the question: What unique_ptr
> properties prevent you from using it for LockingPointer::raw?
> 

The ability to copy-assign a unique_ptr and come out of it with two
pointers both set to the same *-pointer value and guaranteed not to
clobber each others memory.

We can get it to compile by doing raw.reset(o.raw.get()); but that is
documented as resulting in undefined behaviour for both unique_ptr. It
will likely only work for OpenSSL by way of the resulting two unique_ptr
Deleter's doing the unlock semantic instead of actual delete. We need it
to work for non-OpenSSL libraries too and situatinos like xfree() /
tidyFree() where the object deallocator knows nothing about locks.

This is next-stage discussion though. One you are okay with the rest of
the attached patch.


> 
>>> Replacing custom TidyPointer with standard unique_ptr is a welcomed
>>> improvement that can and should be accepted. Whether you commit those
>>> [polished] changes before or while fixing "LockingPointer issues" is
>>> your call. However, this reasoning alone does not make code duplication
>>> necessary or acceptable.
> 
>> Nod. Its not exactly duplication though. It was cut-n-paste, not
>> copy-n-paste. So if anything is duplicated it was beforehand too, just
>> not so obviousy so.
> 
> TidyPointer was certainly duplicating unique_ptr. IIRC, there was a
> valid reason for that functionality duplication -- Squid did not have a
> reliable access to unique_ptr when TidyPointer was written. Note that
> there was no code duplication from Squid point of view; there was a
> "reinventing the wheel" problem instead.

Yes. We agree on that.

> 
> Now, when replacing TidyPointer with the now-available std::unique_ptr
> class, you can no longer use unique_ptr unavailability as an excuse to
> duplicate TidyPointer/unique_ptr functionality in the LockingPointer.
> 

You seem to be saying I should have made LockingPointer inherit from
std::unique_ptr and keep touching its data member for the shared_ptr
things LockingPointer does.


> 
>>>> unique_ptr uses a Functor type.
>>>
>>> This statement is inaccurate in its context -- unique_ptr does accept
>>> [lvalue references to] functions as well. I believe this is by design --
>>> any good template should not care whether it was given a simple function
>>> or a Functor object.
> 
> 
>> Thats how I took the documentation, in fact it says so outright in
>> place. But I was not able to easily get it to accept either a function
>> pointer, or a function type.
> 
> Yeah. I think there are two different problems here, and that obscures
> the solution. I will illustrate using BIGNUM_Pointer as an example:
> 
> A) The Deleter template parameter must be a type, not an object. Thus,
> declaring BIGNUM_Pointer using BN_free_cpp as Deleter does not work:
> BN_free_cpp is not a type but an object.
> 
> B) We do not want to specify the Deleter object explicitly every time we
> are defining our BIGNUM_Pointer objects. Thus, our Deleter type must
> have a default constructor. Pointers to functions do not have default
> constructors. Thus, the Deleter type must be a [DefaultConstructible]
> class and not just a "pointer to function X" type.
> 
> All of the following "typedef ... BIGNUM_Pointer" variants compile in my
> tests:
> 
>   1.  std::unique_ptr<BIGNUM, std::function<decltype(BN_free_cpp)> >
>   2.  std::unique_ptr<BIGNUM, std::function<decltype(BN_free)> >
>   3a. std::unique_ptr<BIGNUM, decltype(&BN_free_cpp) >
>   3b. std::unique_ptr<BIGNUM, void(*)(BIGNUM*) >
> 
> #3a and #3b force you to pass a specific deleter when defining
> BIGNUM_Pointer objects, violating condition (B) above. For example:
>   BIGNUM_Pointer bnp(..., &BN_free_cpp);

That rules #3a and #3b out for me. Too much required extra code.

> 
> #2 might suffer from the same problem that forced us to add the CtoCpp1
> macro. Christos, the author of r11100 that added CtoCpp1, may be able to
> confirm or deny this.

IIRC the issue then was linking with symbol mangling (or not). I think
the C++11 decltype() takes care of that here.

std::function creates a Functor. So as I said unique_ptr uses a Functor
type.

> 
>> ../../../src/ssl/gadgets.h:51:44: note:   expected a type, got
>> ‘Ssl::BN_free_cpp’
> 
> Yes, this is due to (A) above. Solutions #1-3 avoid that problem.
> 
> 
>> If I give it the type instead of function pointer, like so:
>>   typedef std::unique_ptr<BIGNUM, decltype(BN_free_cpp)> BIGNUM_Pointer;
> 
> AFAICT, you want a reference to the function, not the function name
> itself. I do not know why the usual decaying rules do not apply here,
> but perhaps that is not important. #3a solves this but violates (B) as
> discussed above.
> 
> 
>> Asside from that the functor keeps the code looking almost the same as
>> it did before. So a bit less pain for all of us porting things to 3.5 in
>> the next years.
> 
> I hope the above both explains exactly why we need a functor here and
> offers a similar std::function-based solution that does not require
> re-inventing UnaryFunctor.
> 

Yes. Thank you.


Amos
-------------- next part --------------
=== modified file 'src/base/Makefile.am'
--- src/base/Makefile.am	2016-01-15 06:47:59 +0000
+++ src/base/Makefile.am	2016-06-29 03:56:37 +0000
@@ -21,22 +21,21 @@
 	AsyncCallQueue.h \
 	ByteCounter.h \
 	CbcPointer.h \
 	CbDataList.h \
 	CharacterSet.h \
 	CharacterSet.cc \
 	EnumIterator.h \
 	InstanceId.h \
 	Lock.h \
 	LookupTable.h \
 	LruMap.h \
 	Packable.h \
 	PackableStream.h \
 	RegexPattern.cc \
 	RegexPattern.h \
 	RunnersRegistry.cc \
 	RunnersRegistry.h \
 	Subscription.h \
 	TextException.cc \
 	TextException.h \
-	TidyPointer.h \
 	YesNoNone.h

=== removed file 'src/base/TidyPointer.h'
--- src/base/TidyPointer.h	2016-03-31 23:33:45 +0000
+++ src/base/TidyPointer.h	1970-01-01 00:00:00 +0000
@@ -1,69 +0,0 @@
-/*
- * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
- *
- * Squid software is distributed under GPLv2+ license and includes
- * contributions from numerous individuals and organizations.
- * Please see the COPYING and CONTRIBUTORS files for details.
- */
-
-#ifndef SQUID_BASE_TIDYPOINTER_H
-#define SQUID_BASE_TIDYPOINTER_H
-
-/**
- * A pointer that deletes the object it points to when the pointer's owner or
- * context is gone. Similar to std::unique_ptr but without confusing assignment
- * and with a customizable cleanup method. Prevents memory leaks in
- * the presence of exceptions and processing short cuts.
-*/
-template <typename T, void (*DeAllocator)(T *t)> class TidyPointer
-{
-public:
-    /// Delete callback.
-    typedef void DCB (T *t);
-    TidyPointer(T *t = NULL)
-        :   raw(t) {}
-public:
-    bool operator !() const { return !raw; }
-    explicit operator bool() const { return raw; }
-    /// Returns raw and possibly NULL pointer
-    T *get() const { return raw; }
-
-    /// Reset raw pointer - delete last one and save new one.
-    void reset(T *t) {
-        deletePointer();
-        raw = t;
-    }
-
-    /// Forget the raw pointer without freeing it. Become a nil pointer.
-    T *release() {
-        T *ret = raw;
-        raw = NULL;
-        return ret;
-    }
-    /// Deallocate raw pointer.
-    ~TidyPointer() {
-        deletePointer();
-    }
-private:
-    /// Forbidden copy constructor.
-    TidyPointer(TidyPointer<T, DeAllocator> const &);
-    /// Forbidden assigment operator.
-    TidyPointer <T, DeAllocator> & operator = (TidyPointer<T, DeAllocator> const &);
-    /// Deallocate raw pointer. Become a nil pointer.
-    void deletePointer() {
-        if (raw) {
-            DeAllocator(raw);
-        }
-        raw = NULL;
-    }
-    T *raw; ///< pointer to T object or NULL
-};
-
-/// DeAllocator for pointers that need free(3) from the std C library
-template<typename T> void tidyFree(T *p)
-{
-    xfree(p);
-}
-
-#endif // SQUID_BASE_TIDYPOINTER_H
-

=== modified file 'src/security/LockingPointer.h'
--- src/security/LockingPointer.h	2016-03-31 23:33:45 +0000
+++ src/security/LockingPointer.h	2016-06-25 15:23:17 +0000
@@ -1,89 +1,125 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_SRC_SECURITY_LOCKINGPOINTER_H
 #define SQUID_SRC_SECURITY_LOCKINGPOINTER_H
 
-#include "base/TidyPointer.h"
-
 #if USE_OPENSSL
 #if HAVE_OPENSSL_CRYPTO_H
 #include <openssl/crypto.h>
 #endif
 
 // Macro to be used to define the C++ wrapper function of a sk_*_pop_free
 // openssl family functions. The C++ function suffixed with the _free_wrapper
 // extension
 #define sk_free_wrapper(sk_object, argument, freefunction) \
         extern "C++" inline void sk_object ## _free_wrapper(argument a) { \
             sk_object ## _pop_free(a, freefunction); \
         }
 
-#endif
+#endif /* USE_OPENSSL */
 
 // Macro to be used to define the C++ equivalent function of an extern "C"
 // function. The C++ function suffixed with the _cpp extension
 #define CtoCpp1(function, argument) \
         extern "C++" inline void function ## _cpp(argument a) { \
             function(a); \
         }
 
 namespace Security
 {
 
 /**
- * Add SSL locking (a.k.a. reference counting) and assignment to TidyPointer
+ * A pointer that deletes the object it points to when the pointer's owner or
+ * context is gone.
+ * Maintains locking using OpenSSL crypto API when exporting the stored value
+ * between objects.
+ * Prevents memory leaks in the presence of exceptions and processing short
+ * cuts.
  */
 template <typename T, void (*DeAllocator)(T *t), int lock>
-class LockingPointer: public TidyPointer<T, DeAllocator>
+class LockingPointer
 {
 public:
-    typedef TidyPointer<T, DeAllocator> Parent;
+    /// a helper label to simplify this objects API definitions below
     typedef LockingPointer<T, DeAllocator, lock> SelfType;
 
-    explicit LockingPointer(T *t = nullptr): Parent(t) {}
+    /**
+     * Construct directly from a raw pointer.
+     * This action requires that the producer of that pointer has already
+     * created one reference lock for the object pointed to.
+     * Our destructor will do the matching unlock/free.
+     */
+    explicit LockingPointer(T *t = nullptr): raw(t) {}
 
-    explicit LockingPointer(const SelfType &o): Parent() {
-        resetAndLock(o.get());
-    }
+    /// use the custom DeAllocator to unlock and/or free any value still stored.
+    ~LockingPointer() { deletePointer(); }
 
+    // copy semantics are okay only when adding a lock reference
+    explicit LockingPointer(const SelfType &o) : raw(nullptr) { resetAndLock(o.get()); }
     SelfType &operator =(const SelfType & o) {
         resetAndLock(o.get());
         return *this;
     }
 
-#if __cplusplus >= 201103L
-    explicit LockingPointer(LockingPointer<T, DeAllocator, lock> &&o): Parent(o.release()) {
+    // move semantics are definitely okay, when possible
+    explicit LockingPointer(SelfType &&) = default;
+    SelfType &operator =(SelfType &&o) {
+        if (o.get() != raw)
+            reset(o.release());
+        return *this;
     }
 
-    LockingPointer<T, DeAllocator, lock> &operator =(LockingPointer<T, DeAllocator, lock> &&o) {
-        if (o.get() != this->get())
-            this->reset(o.release());
-        return *this;
+    bool operator !() const { return !raw; }
+    explicit operator bool() const { return raw; }
+
+    /// Returns raw and possibly nullptr pointer
+    T *get() const { return raw; }
+
+    /// Reset raw pointer - delete last one and save new one.
+    void reset(T *t) {
+        deletePointer();
+        raw = t;
     }
-#endif
 
     void resetAndLock(T *t) {
         if (t != this->get()) {
             this->reset(t);
 #if USE_OPENSSL
             if (t)
                 CRYPTO_add(&t->references, 1, lock);
 #elif USE_GNUTLS
             // XXX: GnuTLS does not provide locking ?
 #else
             assert(false);
 #endif
         }
     }
+
+    /// Forget the raw pointer without freeing it. Become a nil pointer.
+    T *release() {
+        T *ret = raw;
+        raw = nullptr;
+        return ret;
+    }
+
+private:
+    /// Deallocate raw pointer. Become a nil pointer.
+    void deletePointer() {
+        if (raw)
+            DeAllocator(raw);
+        raw = nullptr;
+    }
+
+    T *raw; ///< pointer to T object or nullptr
 };
 
 } // namespace Security
 
 #endif /* SQUID_SRC_SECURITY_LOCKINGPOINTER_H */
 

=== modified file 'src/security/Session.h'
--- src/security/Session.h	2016-02-13 12:12:10 +0000
+++ src/security/Session.h	2016-06-29 10:44:21 +0000
@@ -1,53 +1,54 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_SRC_SECURITY_SESSION_H
 #define SQUID_SRC_SECURITY_SESSION_H
 
-// LockingPointer.h instead of TidyPointer.h for CtoCpp1()
 #include "security/LockingPointer.h"
 
+#include <memory>
+
 #if USE_OPENSSL
 #if HAVE_OPENSSL_SSL_H
 #include <openssl/ssl.h>
 #endif
 #endif
 
 #if USE_GNUTLS
 #if HAVE_GNUTLS_GNUTLS_H
 #include <gnutls/gnutls.h>
 #endif
 #endif
 
 namespace Security {
 
 #if USE_OPENSSL
 typedef SSL* SessionPtr;
 CtoCpp1(SSL_free, SSL *);
 typedef LockingPointer<SSL, Security::SSL_free_cpp, CRYPTO_LOCK_SSL> SessionPointer;
 
 #elif USE_GNUTLS
 typedef gnutls_session_t SessionPtr;
-CtoCpp1(gnutls_deinit, gnutls_session_t);
 // TODO: Convert to Locking pointer.
 // Locks can be implemented attaching locks counter to gnutls_session_t
 // objects using the gnutls_session_set_ptr()/gnutls_session_get_ptr ()
 // library functions
-typedef TidyPointer<struct gnutls_session_int, Security::gnutls_deinit_cpp> SessionPointer;
+typedef std::unique_ptr<struct gnutls_session_int, std::function<decltype(gnutls_deinit)>> SessionPointer;
 
 #else
 // use void* so we can check against NULL
 typedef void* SessionPtr;
-typedef TidyPointer<void, nullptr> SessionPointer;
+// use nullptr_t so default_delete works
+typedef std::unique_ptr<std::nullptr_t> SessionPointer;
 
 #endif
 
 } // namespace Security
 
 #endif /* SQUID_SRC_SECURITY_SESSION_H */
 

=== modified file 'src/security/cert_generators/file/certificate_db.cc'
--- src/security/cert_generators/file/certificate_db.cc	2016-02-13 16:10:26 +0000
+++ src/security/cert_generators/file/certificate_db.cc	2016-06-29 03:57:52 +0000
@@ -1,33 +1,34 @@
 /*
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #include "squid.h"
 #include "security/cert_generators/file/certificate_db.h"
 
 #include <cerrno>
 #include <fstream>
+#include <memory>
 #include <stdexcept>
 #if HAVE_SYS_STAT_H
 #include <sys/stat.h>
 #endif
 #if HAVE_SYS_FILE_H
 #include <sys/file.h>
 #endif
 #if HAVE_FCNTL_H
 #include <fcntl.h>
 #endif
 
 #define HERE "(security_file_certgen) " << __FILE__ << ':' << __LINE__ << ": "
 
 Ssl::Lock::Lock(std::string const &aFilename) :
     filename(aFilename),
 #if _SQUID_WINDOWS_
     hFile(INVALID_HANDLE_VALUE)
 #else
     fd(-1)
 #endif
@@ -235,94 +236,93 @@
     return strcmp(aa, bb);
 }
 
 unsigned long Ssl::CertificateDb::index_name_hash(const char **a) {
     return(lh_strhash(a[Ssl::CertificateDb::cnlName]));
 }
 
 int Ssl::CertificateDb::index_name_cmp(const char **a, const char **b) {
     return(strcmp(a[Ssl::CertificateDb::cnlName], b[CertificateDb::cnlName]));
 }
 
 const std::string Ssl::CertificateDb::db_file("index.txt");
 const std::string Ssl::CertificateDb::cert_dir("certs");
 const std::string Ssl::CertificateDb::size_file("size");
 
 Ssl::CertificateDb::CertificateDb(std::string const & aDb_path, size_t aMax_db_size, size_t aFs_block_size)
     :  db_path(aDb_path),
        db_full(aDb_path + "/" + db_file),
        cert_full(aDb_path + "/" + cert_dir),
        size_full(aDb_path + "/" + size_file),
-       db(NULL),
        max_db_size(aMax_db_size),
        fs_block_size((aFs_block_size ? aFs_block_size : 2048)),
        dbLock(db_full),
        enabled_disk_store(true) {
     if (db_path.empty() && !max_db_size)
         enabled_disk_store = false;
     else if ((db_path.empty() && max_db_size) || (!db_path.empty() && !max_db_size))
         throw std::runtime_error("security_file_certgen is missing the required parameter. There should be -s and -M parameters together.");
 }
 
 bool Ssl::CertificateDb::find(std::string const & host_name, Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey) {
     const Locker locker(dbLock, Here);
     load();
     return pure_find(host_name, cert, pkey);
 }
 
 bool Ssl::CertificateDb::purgeCert(std::string const & key) {
     const Locker locker(dbLock, Here);
     load();
     if (!db)
         return false;
 
     if (!deleteByHostname(key))
         return false;
 
     save();
     return true;
 }
 
 bool Ssl::CertificateDb::addCertAndPrivateKey(Security::CertPointer & cert, Ssl::EVP_PKEY_Pointer & pkey, std::string const & useName) {
     const Locker locker(dbLock, Here);
     load();
     if (!db || !cert || !pkey)
         return false;
     Row row;
     ASN1_INTEGER * ai = X509_get_serialNumber(cert.get());
     std::string serial_string;
     Ssl::BIGNUM_Pointer serial(ASN1_INTEGER_to_BN(ai, NULL));
     {
-        TidyPointer<char, tidyFree> hex_bn(BN_bn2hex(serial.get()));
+        std::unique_ptr<char, std::function<decltype(xfree)>> hex_bn(BN_bn2hex(serial.get()));
         serial_string = std::string(hex_bn.get());
     }
     row.setValue(cnlSerial, serial_string.c_str());
     char ** rrow = TXT_DB_get_by_index(db.get(), cnlSerial, row.getRow());
     // We are creating certificates with unique serial numbers. If the serial
     // number is found in the database, the same certificate is already stored.
     if (rrow != NULL) {
         // TODO: check if the stored row is valid.
         return true;
     }
 
     {
-        TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
+        std::unique_ptr<char, std::function<decltype(xfree)>> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), nullptr, 0));
         Security::CertPointer findCert;
         Ssl::EVP_PKEY_Pointer findPkey;
         if (pure_find(useName.empty() ? subject.get() : useName, findCert, findPkey)) {
             // Replace with database certificate
             cert.reset(findCert.release());
             pkey.reset(findPkey.release());
             return true;
         }
         // pure_find may fail because the entry is expired, or because the
         // certs file is corrupted. Remove any entry with given hostname
         deleteByHostname(useName.empty() ? subject.get() : useName);
     }
 
     // check db size while trying to minimize calls to size()
     size_t dbSize = size();
     if ((dbSize == 0 && hasRows()) ||
             (dbSize > 0 && !hasRows()) ||
             (dbSize >  10 * max_db_size)) {
         // Invalid database size, rebuild
         dbSize = rebuildSize();
@@ -331,41 +331,41 @@
         dbSize = size(); // get the current database size
         // and try to find another invalid certificate if needed
     }
     // there are no more invalid ones, but there must be valid certificates
     while (dbSize > max_db_size) {
         if (!deleteOldestCertificate()) {
             rebuildSize(); // No certificates in database.Update the size file.
             save(); // Some entries may have been removed. Update the index file.
             return false; // errors prevented us from freeing enough space
         }
         dbSize = size(); // get the current database size
     }
 
     row.setValue(cnlType, "V");
     ASN1_UTCTIME * tm = X509_get_notAfter(cert.get());
     row.setValue(cnlExp_date, std::string(reinterpret_cast<char *>(tm->data), tm->length).c_str());
     row.setValue(cnlFile, "unknown");
     if (!useName.empty())
         row.setValue(cnlName, useName.c_str());
     else {
-        TidyPointer<char, tidyFree> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), NULL, 0));
+        std::unique_ptr<char, std::function<decltype(xfree)>> subject(X509_NAME_oneline(X509_get_subject_name(cert.get()), nullptr, 0));
         row.setValue(cnlName, subject.get());
     }
 
     if (!TXT_DB_insert(db.get(), row.getRow())) {
         // failed to add index (???) but we may have already modified
         // the database so save before exit
         save();
         return false;
     }
     rrow = row.getRow();
     row.reset();
 
     std::string filename(cert_full + "/" + serial_string + ".pem");
     if (!writeCertAndPrivateKeyToFile(cert, pkey, filename.c_str())) {
         //remove row from txt_db and save
         sq_TXT_DB_delete(db.get(), (const char **)rrow);
         save();
         return false;
     }
     addSize(filename);

=== modified file 'src/security/forward.h'
--- src/security/forward.h	2016-01-01 00:12:18 +0000
+++ src/security/forward.h	2016-06-24 13:09:31 +0000
@@ -2,40 +2,50 @@
  * Copyright (C) 1996-2016 The Squid Software Foundation and contributors
  *
  * Squid software is distributed under GPLv2+ license and includes
  * contributions from numerous individuals and organizations.
  * Please see the COPYING and CONTRIBUTORS files for details.
  */
 
 #ifndef SQUID_SRC_SECURITY_FORWARD_H
 #define SQUID_SRC_SECURITY_FORWARD_H
 
 #include "security/Context.h"
 #include "security/Session.h"
 
 #if USE_GNUTLS
 #if HAVE_GNUTLS_X509_H
 #include <gnutls/x509.h>
 #endif
 #endif
 #include <list>
 
+#if USE_OPENSSL
+// Macro to be used to define the C++ wrapper functor of the sk_*_pop_free
+// OpenSSL family of functions. The C++ functor is suffixed with the _free_wrapper
+// extension
+#define sk_dtor_wrapper(sk_object, argument_type, freefunction) \
+        struct sk_object ## _free_wrapper { \
+            void operator()(argument_type a) { sk_object ## _pop_free(a, freefunction); } \
+        }
+#endif /* USE_OPENSSL */
+
 /* flags a SSL connection can be configured with */
 #define SSL_FLAG_NO_DEFAULT_CA      (1<<0)
 #define SSL_FLAG_DELAYED_AUTH       (1<<1)
 #define SSL_FLAG_DONT_VERIFY_PEER   (1<<2)
 #define SSL_FLAG_DONT_VERIFY_DOMAIN (1<<3)
 #define SSL_FLAG_NO_SESSION_REUSE   (1<<4)
 #define SSL_FLAG_VERIFY_CRL         (1<<5)
 #define SSL_FLAG_VERIFY_CRL_ALL     (1<<6)
 
 /// Network/connection security abstraction layer
 namespace Security
 {
 
 class EncryptorAnswer;
 class PeerOptions;
 class ServerOptions;
 
 #if USE_OPENSSL
 CtoCpp1(X509_free, X509 *)
 typedef Security::LockingPointer<X509, X509_free_cpp, CRYPTO_LOCK_X509> CertPointer;

=== modified file 'src/ssl/gadgets.h'
--- src/ssl/gadgets.h	2016-06-14 15:56:12 +0000
+++ src/ssl/gadgets.h	2016-06-29 03:54:45 +0000
@@ -22,87 +22,75 @@
 
 namespace Ssl
 {
 /**
  \defgroup SslCrtdSslAPI SSL certificate generator API
  These functions must not depend on Squid runtime code such as debug()
  because they are used by security_file_certgen helper.
  */
 
 #if SQUID_USE_CONST_SSL_METHOD
 typedef const SSL_METHOD * ContextMethod;
 #else
 typedef SSL_METHOD * ContextMethod;
 #endif
 
 #if !defined(SQUID_SSL_SIGN_HASH_IF_NONE)
 #define SQUID_SSL_SIGN_HASH_IF_NONE "sha256"
 #endif
 
 /**
- \ingroup SslCrtdSslAPI
- * TidyPointer typedefs for  common SSL objects
+ * std::unique_ptr typedefs for common SSL objects
  */
-sk_free_wrapper(sk_X509, STACK_OF(X509) *, X509_free)
-typedef TidyPointer<STACK_OF(X509), sk_X509_free_wrapper> X509_STACK_Pointer;
+sk_dtor_wrapper(sk_X509, STACK_OF(X509) *, X509_free);
+typedef std::unique_ptr<STACK_OF(X509), sk_X509_free_wrapper> X509_STACK_Pointer;
 
 CtoCpp1(EVP_PKEY_free, EVP_PKEY *)
 typedef Security::LockingPointer<EVP_PKEY, EVP_PKEY_free_cpp, CRYPTO_LOCK_EVP_PKEY> EVP_PKEY_Pointer;
 
-CtoCpp1(BN_free, BIGNUM *)
-typedef TidyPointer<BIGNUM, BN_free_cpp> BIGNUM_Pointer;
+typedef std::unique_ptr<BIGNUM, std::function<decltype(BN_free)>> BIGNUM_Pointer;
 
-CtoCpp1(BIO_free, BIO *)
-typedef TidyPointer<BIO, BIO_free_cpp> BIO_Pointer;
+typedef std::unique_ptr<BIO, std::function<decltype(BIO_free)>> BIO_Pointer;
 
-CtoCpp1(ASN1_INTEGER_free, ASN1_INTEGER *)
-typedef TidyPointer<ASN1_INTEGER, ASN1_INTEGER_free_cpp> ASN1_INT_Pointer;
+typedef std::unique_ptr<ASN1_INTEGER, std::function<decltype(ASN1_INTEGER_free)>> ASN1_INT_Pointer;
 
-CtoCpp1(ASN1_OCTET_STRING_free, ASN1_OCTET_STRING *)
-typedef TidyPointer<ASN1_OCTET_STRING, ASN1_OCTET_STRING_free_cpp> ASN1_OCTET_STRING_Pointer;
+typedef std::unique_ptr<ASN1_OCTET_STRING, std::function<decltype(ASN1_OCTET_STRING_free)>> ASN1_OCTET_STRING_Pointer;
 
-CtoCpp1(TXT_DB_free, TXT_DB *)
-typedef TidyPointer<TXT_DB, TXT_DB_free_cpp> TXT_DB_Pointer;
+typedef std::unique_ptr<TXT_DB, std::function<decltype(TXT_DB_free)>> TXT_DB_Pointer;
 
-CtoCpp1(X509_NAME_free, X509_NAME *)
-typedef TidyPointer<X509_NAME, X509_NAME_free_cpp> X509_NAME_Pointer;
+typedef std::unique_ptr<X509_NAME, std::function<decltype(X509_NAME_free)>> X509_NAME_Pointer;
 
-CtoCpp1(RSA_free, RSA *)
-typedef TidyPointer<RSA, RSA_free_cpp> RSA_Pointer;
+typedef std::unique_ptr<RSA, std::function<decltype(RSA_free)>> RSA_Pointer;
 
-CtoCpp1(X509_REQ_free, X509_REQ *)
-typedef TidyPointer<X509_REQ, X509_REQ_free_cpp> X509_REQ_Pointer;
+typedef std::unique_ptr<X509_REQ, std::function<decltype(X509_REQ_free)>> X509_REQ_Pointer;
 
-sk_free_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free)
-typedef TidyPointer<STACK_OF(X509_NAME), sk_X509_NAME_free_wrapper> X509_NAME_STACK_Pointer;
+sk_dtor_wrapper(sk_X509_NAME, STACK_OF(X509_NAME) *, X509_NAME_free);
+typedef std::unique_ptr<STACK_OF(X509_NAME), sk_X509_NAME_free_wrapper> X509_NAME_STACK_Pointer;
 
-CtoCpp1(AUTHORITY_KEYID_free, AUTHORITY_KEYID *)
-typedef TidyPointer<AUTHORITY_KEYID, AUTHORITY_KEYID_free_cpp> AUTHORITY_KEYID_Pointer;
+typedef std::unique_ptr<AUTHORITY_KEYID, std::function<decltype(AUTHORITY_KEYID_free)>> AUTHORITY_KEYID_Pointer;
 
-sk_free_wrapper(sk_GENERAL_NAME, STACK_OF(GENERAL_NAME) *, GENERAL_NAME_free)
-typedef TidyPointer<STACK_OF(GENERAL_NAME), sk_GENERAL_NAME_free_wrapper> GENERAL_NAME_STACK_Pointer;
+sk_dtor_wrapper(sk_GENERAL_NAME, STACK_OF(GENERAL_NAME) *, GENERAL_NAME_free);
+typedef std::unique_ptr<STACK_OF(GENERAL_NAME), sk_GENERAL_NAME_free_wrapper> GENERAL_NAME_STACK_Pointer;
 
-CtoCpp1(GENERAL_NAME_free, GENERAL_NAME *)
-typedef TidyPointer<GENERAL_NAME, GENERAL_NAME_free_cpp> GENERAL_NAME_Pointer;
+typedef std::unique_ptr<GENERAL_NAME, std::function<decltype(GENERAL_NAME_free)>> GENERAL_NAME_Pointer;
 
-CtoCpp1(X509_EXTENSION_free, X509_EXTENSION *)
-typedef TidyPointer<X509_EXTENSION, X509_EXTENSION_free_cpp> X509_EXTENSION_Pointer;
+typedef std::unique_ptr<X509_EXTENSION, std::function<decltype(X509_EXTENSION_free)>> X509_EXTENSION_Pointer;
 
 /**
  \ingroup SslCrtdSslAPI
  * Create 1024 bits rsa key.
  */
 EVP_PKEY * createSslPrivateKey();
 
 /**
  \ingroup SslCrtdSslAPI
  * Write private key and SSL certificate to memory.
  */
 bool writeCertAndPrivateKeyToMemory(Security::CertPointer const & cert, EVP_PKEY_Pointer const & pkey, std::string & bufferToWrite);
 
 /**
  \ingroup SslCrtdSslAPI
  * Append SSL certificate to bufferToWrite.
  */
 bool appendCertToMemory(Security::CertPointer const & cert, std::string & bufferToWrite);
 
 /**



More information about the squid-dev mailing list