[squid-dev] [PATCH] TidyPointer removal
Amos Jeffries
squid3 at treenet.co.nz
Tue Jun 28 14:52:54 UTC 2016
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. Without
that LockingPointer would inherit from std::unique_ptr.
> 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.
Thats also why I wanted to pause here and get an audit.
>
>> The resulting situation is that TidyPointer is a stand-alone Pointer
>> type that duplicates the std::unique_ptr API so closely it can almost be
>> a drop-in replacement. The remaining difference is that TidyPointer took
>> a function pointer whereas unique_ptr uses a Functor type.
>>
>> Adding a UniaryFunctor(function, arg) macro in src/base/MakeFunctor.h to
>> replace the CtoCpp1(function, arg) macro used by TidyPointer allows us
>> to remove TidyPointer completely from Squid-4.
>
> 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.
>
>> The LockingPointer still has the issues which led us down the path to
>> getting here. I've chosen to stop and submit for review at this point to
>> ensure we still have a fully working Squid before going further down
>> this trail.
>
> Stopping here is perfectly fine, but please do not duplicate
> TidyPointer/unique_ptr code/functionality. That duplication did not
> exist before, so you cannot argue that it is one of the old
> LockingPointer issues that you will fix later.
>
>
>> 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.
Digging into both the docs and the code it appears there is a special
type defined as Deleter that the parameter uses. A Deleter is a Functor
class AFAICS, not a function pointer and what is needed is a type that
can cast into a reference to a Deleter. So it cycles back to something
that can be implicitly cast to a Functor.
Function pointer is apparently supposed to have that implicit cast
property. But...
If I revert the BIGNUM_Pointer definition in ssl/gadgets.h to using
CtoCpp1() like trunk, but with std::unique_ptr I get this:
In file included from ../../../src/ssl/support.h:19:0,
from ../../../src/ssl/bio.cc:12:
../../../src/ssl/gadgets.h:51:44: error: type/value mismatch at argument
2 in template parameter list for ‘template<class _Tp, class _Dp> class
std::unique_ptr’
typedef std::unique_ptr<BIGNUM, BN_free_cpp> BIGNUM_Pointer;
^
../../../src/ssl/gadgets.h:51:44: note: expected a type, got
‘Ssl::BN_free_cpp’
If I give it the type instead of function pointer, like so:
typedef std::unique_ptr<BIGNUM, decltype(BN_free_cpp)> BIGNUM_Pointer;
This comes out:
/usr/include/c++/5/tuple: In instantiation of ‘struct
std::_Head_base<1u, void(bignum_st*), false>’:
/usr/include/c++/5/tuple:339:12: required from ‘struct
std::_Tuple_impl<1u, void(bignum_st*)>’
/usr/include/c++/5/tuple:180:12: required from ‘struct
std::_Tuple_impl<0u, bignum_st*, void(bignum_st*)>’
/usr/include/c++/5/tuple:596:11: required from ‘class
std::tuple<bignum_st*, void(bignum_st*)>’
/usr/include/c++/5/bits/unique_ptr.h:147:57: required from ‘class
std::unique_ptr<bignum_st, void(bignum_st*)>’
../../../src/ssl/gadgets.cc:43:28: required from here
/usr/include/c++/5/tuple:147:13: error: field ‘std::_Head_base<1u,
void(bignum_st*), false>::_M_head_impl’ invalidly declared function type
_Head _M_head_impl;
^
In file included from /usr/include/c++/5/memory:81:0,
from /usr/include/cppunit/extensions/HelperMacros.h:16,
from ../../../compat/cppunit.h:16,
from ../../../compat/compat.h:131,
from ../../../include/squid.h:43,
from ../../../src/ssl/gadgets.cc:9:
/usr/include/c++/5/bits/unique_ptr.h: In instantiation of
‘std::unique_ptr<_Tp, _Dp>::unique_ptr(std::unique_ptr<_Tp,
_Dp>::pointer) [with _Tp = bignum_st; _Dp = void(bignum_st*);
std::unique_ptr<_Tp, _Dp>::pointer = bignum_st*]’:
../../../src/ssl/gadgets.cc:43:36: required from here
/usr/include/c++/5/bits/unique_ptr.h:170:33: error: value-initialization
of function type ‘std::unique_ptr<bignum_st,
void(bignum_st*)>::deleter_type {aka void(bignum_st*)}’
: _M_t(__p, deleter_type())
^
In file included from /usr/include/c++/5/functional:55:0,
from /usr/include/cppunit/portability/CppUnitSet.h:7,
from
/usr/include/cppunit/extensions/TestFactoryRegistry.h:11,
from /usr/include/cppunit/extensions/AutoRegisterSuite.h:5,
from /usr/include/cppunit/extensions/HelperMacros.h:11,
from ../../../compat/cppunit.h:16,
from ../../../compat/compat.h:131,
from ../../../include/squid.h:43,
from ../../../src/ssl/gadgets.cc:9:
/usr/include/c++/5/tuple: In instantiation of ‘static constexpr _Head&
std::_Head_base<_Idx, _Head, false>::_M_head(std::_Head_base<_Idx,
_Head, false>&) [with unsigned int _Idx = 1u; _Head = void(bignum_st*)]’:
/usr/include/c++/5/tuple:347:65: required from ‘static constexpr
_Head& std::_Tuple_impl<_Idx, _Head>::_M_head(std::_Tuple_impl<_Idx,
_Head>&) [with unsigned int _Idx = 1u; _Head = void(bignum_st*)]’
/usr/include/c++/5/tuple:822:56: required from ‘constexpr _Head&
std::__get_helper(std::_Tuple_impl<_Idx, _Head, _Tail ...>&) [with
unsigned int __i = 1u; _Head = void(bignum_st*); _Tail = {}]’
/usr/include/c++/5/tuple:833:36: required from ‘constexpr
std::__tuple_element_t<__i, std::tuple<_Elements ...> >&
std::get(std::tuple<_Elements ...>&) [with unsigned int __i = 1u;
_Elements = {bignum_st*, void(bignum_st*)}; std::__tuple_element_t<__i,
std::tuple<_Elements ...> > = void(bignum_st*)]’
/usr/include/c++/5/bits/unique_ptr.h:310:27: required from
‘std::unique_ptr<_Tp, _Dp>::deleter_type& std::unique_ptr<_Tp,
_Dp>::get_deleter() [with _Tp = bignum_st; _Dp = void(bignum_st*);
std::unique_ptr<_Tp, _Dp>::deleter_type = void(bignum_st*)]’
/usr/include/c++/5/bits/unique_ptr.h:236:15: required from
‘std::unique_ptr<_Tp, _Dp>::~unique_ptr() [with _Tp = bignum_st; _Dp =
void(bignum_st*)]’
../../../src/ssl/gadgets.cc:43:36: required from here
/usr/include/c++/5/tuple:142:54: error: invalid initialization of
reference of type ‘void (&)(bignum_st*)’ from expression of type ‘void
(*)(bignum_st*)’
_M_head(_Head_base& __b) noexcept { return __b._M_head_impl; }
^
/usr/include/c++/5/tuple:142:68: error: body of constexpr function
‘static constexpr _Head& std::_Head_base<_Idx, _Head,
false>::_M_head(std::_Head_base<_Idx, _Head, false>&) [with unsigned int
_Idx = 1u; _Head = void(bignum_st*)]’ not a return-statement
_M_head(_Head_base& __b) noexcept { return __b._M_head_impl; }
^
I suspect that what it wants is either a class with delete operator. Or
a pointer to a C++ delete operator for the type, or something messy like
that.
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.
>
>> +/// DeAllocator functor for pointers that need free(3) from the std C library
>> +UniaryFunctor(xfree, char *);
>
> Where is xfree_functor used? I only see xfree_char() which is defined
> without using UniaryFunctor.
>
Fixed.
>
>> +// Macro to be used to define a C++ functor of an extern "C"
>> +// function.
>
> I do not see anything in this macro that would make it specific to
> extern "C" functions. I think it works with any function. The old
> CtoCpp1() macro was needed to convert "extern C" functions to "extern
> C++" functions; the [ugly] macro description from r11100 "worked"
> because the macro changed only one aspect -- the linking style changed
> from C to C++. Here, you are changing two aspects at once, resulting in
> a far less helpful comment IMO.
Removed the 'extern "C"' mentions.
>
> Combined, these observations related to adding functors lead me to
> suspect that we do not need to replace CtoCpp1 with UniaryFunctor. What
> errors do you get when building with CtoCpp1 instead of UniaryFunctor?
see above.
>
> Similarly, is there something wrong with tidyFree() that forces you to
> add xfree_char() [with a botched description]?
>
It was leftovers from the first attempt at functor. Fixed to use the
UnaryFunctor macro.
>
>> +#define UniaryFunctor(function, argument_type) \
>
> s/Uniary/Unary/ to fix spelling
>
Fixed.
>
> I hope to post more polishing comments as well, but the code duplication
> and functor issues should probably be resolved first.
>
Nod.
I considered making the UnaryFunctor() macro into a template instead,
but that also meant a fair bit more fluff in the code adding pain to
backports. We should leave that a few years I think.
>
> Thank you,
>
> Alex.
> P.S. If you can post larger context diffs (e.g., -U30), please do so,
> but there is no need to repost the last patch.
>
Updated patch attached in the larger format.
Amos
-------------- next part --------------
=== renamed file 'src/base/TidyPointer.h' => 'src/base/MakeFunctor.h'
--- src/base/TidyPointer.h 2016-03-31 23:33:45 +0000
+++ src/base/MakeFunctor.h 2016-06-28 12:35:41 +0000
@@ -1,69 +1,23 @@
/*
* 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
+#ifndef SQUID_BASE_MAKEFUNCTOR_H
+#define SQUID_BASE_MAKEFUNCTOR_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);
+// Macro to be used to define a C++ functor for a function with one argument.
+// The functor is suffixed with the _functor extension
+#define UnaryFunctor(function, argument_type) \
+ struct function ## _functor { \
+ void operator()(argument_type a) { function(a); } \
}
- 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);
-}
+/// DeAllocator functor for pointers that need free(3)
+UnaryFunctor(xfree, char *);
-#endif // SQUID_BASE_TIDYPOINTER_H
+#endif // SQUID_BASE_MAKEFUNCTOR_H
=== modified file 'src/base/Makefile.am'
--- src/base/Makefile.am 2016-01-15 06:47:59 +0000
+++ src/base/Makefile.am 2016-06-25 18:03:03 +0000
@@ -12,31 +12,31 @@
libbase_la_SOURCES = \
AsyncCall.cc \
AsyncCall.h \
AsyncCbdataCalls.h \
AsyncJob.h \
AsyncJob.cc \
AsyncJobCalls.h \
AsyncCallQueue.cc \
AsyncCallQueue.h \
ByteCounter.h \
CbcPointer.h \
CbDataList.h \
CharacterSet.h \
CharacterSet.cc \
EnumIterator.h \
InstanceId.h \
Lock.h \
LookupTable.h \
LruMap.h \
+ MakeFunctor.h \
Packable.h \
PackableStream.h \
RegexPattern.cc \
RegexPattern.h \
RunnersRegistry.cc \
RunnersRegistry.h \
Subscription.h \
TextException.cc \
TextException.h \
- TidyPointer.h \
YesNoNone.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-28 12:32:52 +0000
@@ -1,53 +1,56 @@
/*
* 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 "base/MakeFunctor.h"
#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);
+UnaryFunctor(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, Security::gnutls_deinit_functor> 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-28 12:36:28 +0000
@@ -1,33 +1,35 @@
/*
* 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 "base/MakeFunctor.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 +237,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, xfree_functor> 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, xfree_functor> 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 +332,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, xfree_functor> 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-28 12:32:45 +0000
@@ -22,87 +22,86 @@
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;
+UnaryFunctor(BN_free, BIGNUM *);
+typedef std::unique_ptr<BIGNUM, BN_free_functor> BIGNUM_Pointer;
-CtoCpp1(BIO_free, BIO *)
-typedef TidyPointer<BIO, BIO_free_cpp> BIO_Pointer;
+UnaryFunctor(BIO_free, BIO *);
+typedef std::unique_ptr<BIO, BIO_free_functor> BIO_Pointer;
-CtoCpp1(ASN1_INTEGER_free, ASN1_INTEGER *)
-typedef TidyPointer<ASN1_INTEGER, ASN1_INTEGER_free_cpp> ASN1_INT_Pointer;
+UnaryFunctor(ASN1_INTEGER_free, ASN1_INTEGER *);
+typedef std::unique_ptr<ASN1_INTEGER, ASN1_INTEGER_free_functor> 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;
+UnaryFunctor(ASN1_OCTET_STRING_free, ASN1_OCTET_STRING *);
+typedef std::unique_ptr<ASN1_OCTET_STRING, ASN1_OCTET_STRING_free_functor> ASN1_OCTET_STRING_Pointer;
-CtoCpp1(TXT_DB_free, TXT_DB *)
-typedef TidyPointer<TXT_DB, TXT_DB_free_cpp> TXT_DB_Pointer;
+UnaryFunctor(TXT_DB_free, TXT_DB *);
+typedef std::unique_ptr<TXT_DB, TXT_DB_free_functor> TXT_DB_Pointer;
-CtoCpp1(X509_NAME_free, X509_NAME *)
-typedef TidyPointer<X509_NAME, X509_NAME_free_cpp> X509_NAME_Pointer;
+UnaryFunctor(X509_NAME_free, X509_NAME *);
+typedef std::unique_ptr<X509_NAME, X509_NAME_free_functor> X509_NAME_Pointer;
-CtoCpp1(RSA_free, RSA *)
-typedef TidyPointer<RSA, RSA_free_cpp> RSA_Pointer;
+UnaryFunctor(RSA_free, RSA *);
+typedef std::unique_ptr<RSA, RSA_free_functor> RSA_Pointer;
-CtoCpp1(X509_REQ_free, X509_REQ *)
-typedef TidyPointer<X509_REQ, X509_REQ_free_cpp> X509_REQ_Pointer;
+UnaryFunctor(X509_REQ_free, X509_REQ *);
+typedef std::unique_ptr<X509_REQ, X509_REQ_free_functor> 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;
+UnaryFunctor(AUTHORITY_KEYID_free, AUTHORITY_KEYID *);
+typedef std::unique_ptr<AUTHORITY_KEYID, AUTHORITY_KEYID_free_functor> 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;
+UnaryFunctor(GENERAL_NAME_free, GENERAL_NAME *);
+typedef std::unique_ptr<GENERAL_NAME, GENERAL_NAME_free_functor> GENERAL_NAME_Pointer;
-CtoCpp1(X509_EXTENSION_free, X509_EXTENSION *)
-typedef TidyPointer<X509_EXTENSION, X509_EXTENSION_free_cpp> X509_EXTENSION_Pointer;
+UnaryFunctor(X509_EXTENSION_free, X509_EXTENSION *);
+typedef std::unique_ptr<X509_EXTENSION, X509_EXTENSION_free_functor> 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