[squid-dev] [PATCH] cleanup removal of needless get()

Amos Jeffries squid3 at treenet.co.nz
Sat Oct 1 05:05:15 UTC 2016


The SSL code in particular contains a lot of calls to the get() on
various Pointer objects from the general code.

Now that C++11 gives our Pointer better boolean operators, and
dereference oerators have been added. A bunch of these calls are not
needing to be explicit. Remove them and use the available class
operators for more easily read code.

NOTE that this does not include the get() to produce raw-pointers for
library API calls.

Also, this patch does not seek to be all-inclusive sweepign change. It
was built using a grep for if-statements using get() output as their
sole boolean condition and polishing the code surrounding. So there are
likely many other uses that can be removed in other patches. This is
just incremental polish.

Amos
-------------- next part --------------
=== modified file 'src/acl/ServerCertificate.cc'
--- src/acl/ServerCertificate.cc	2016-01-24 17:41:43 +0000
+++ src/acl/ServerCertificate.cc	2016-09-23 00:40:27 +0000
@@ -21,16 +21,16 @@
 int
 ACLServerCertificateStrategy::match(ACLData<MatchType> * &data, ACLFilledChecklist *checklist, ACLFlags &)
 {
-    X509 *cert = NULL;
-    if (checklist->serverCert.get())
-        cert = checklist->serverCert.get();
+    Security::CertPointer cert;
+    if (checklist->serverCert)
+        cert = checklist->serverCert;
     else if (checklist->conn() != NULL && checklist->conn()->serverBump())
-        cert = checklist->conn()->serverBump()->serverCert.get();
+        cert = checklist->conn()->serverBump()->serverCert;
 
     if (!cert)
         return 0;
 
-    return data->match(cert);
+    return data->match(cert.get());
 }
 
 ACLServerCertificateStrategy *

=== modified file 'src/client_side.cc'
--- src/client_side.cc	2016-09-22 14:21:12 +0000
+++ src/client_side.cc	2016-09-23 00:53:09 +0000
@@ -2901,10 +2901,10 @@
 
     // fake certificate adaptation requires bump-server-first mode
     if (!sslServerBump) {
-        assert(port->signingCert.get());
-        certProperties.signWithX509.resetAndLock(port->signingCert.get());
-        if (port->signPkey.get())
-            certProperties.signWithPkey.resetAndLock(port->signPkey.get());
+        assert(port->signingCert);
+        certProperties.signWithX509 = port->signingCert;
+        if (port->signPkey)
+            certProperties.signWithPkey = port->signPkey;
         certProperties.signAlgorithm = Ssl::algSignTrusted;
         return;
     }
@@ -2968,15 +2968,15 @@
     assert(certProperties.signAlgorithm != Ssl::algSignEnd);
 
     if (certProperties.signAlgorithm == Ssl::algSignUntrusted) {
-        assert(port->untrustedSigningCert.get());
-        certProperties.signWithX509.resetAndLock(port->untrustedSigningCert.get());
-        certProperties.signWithPkey.resetAndLock(port->untrustedSignPkey.get());
+        assert(port->untrustedSigningCert);
+        certProperties.signWithX509 = port->untrustedSigningCert;
+        certProperties.signWithPkey = port->untrustedSignPkey;
     } else {
-        assert(port->signingCert.get());
-        certProperties.signWithX509.resetAndLock(port->signingCert.get());
+        assert(port->signingCert);
+        certProperties.signWithX509 = port->signingCert;
 
-        if (port->signPkey.get())
-            certProperties.signWithPkey.resetAndLock(port->signPkey.get());
+        if (port->signPkey)
+            certProperties.signWithPkey = port->signPkey;
     }
     signAlgorithm = certProperties.signAlgorithm;
 
@@ -3272,7 +3272,7 @@
 {
     // normally we can splice here, because we just got client hello message
 
-    if (fd_table[clientConnection->fd].ssl.get()) {
+    if (fd_table[clientConnection->fd].ssl) {
         // Restore default read methods
         fd_table[clientConnection->fd].read_method = &default_read_method;
         fd_table[clientConnection->fd].write_method = &default_write_method;

=== modified file 'src/security/ServerOptions.cc'
--- src/security/ServerOptions.cc	2016-09-22 14:21:12 +0000
+++ src/security/ServerOptions.cc	2016-09-23 00:51:05 +0000
@@ -202,7 +202,7 @@
 
     // set DH parameters into the server context
 #if USE_OPENSSL
-    if (parsedDhParams.get()) {
+    if (parsedDhParams) {
         SSL_CTX_set_tmp_dh(ctx.get(), parsedDhParams.get());
     }
 #endif

=== modified file 'src/security/cert_generators/file/security_file_certgen.cc'
--- src/security/cert_generators/file/security_file_certgen.cc	2016-08-19 06:55:38 +0000
+++ src/security/cert_generators/file/security_file_certgen.cc	2016-09-23 00:50:57 +0000
@@ -200,7 +200,7 @@
         error = err.what();
     }
 
-    if (cert.get()) {
+    if (cert) {
         if (!Ssl::certificateMatchesProperties(cert.get(), certProperties)) {
             // The certificate changed (renewed or other reason).
             // Generete a new one with the updated fields.

=== modified file 'src/ssl/ErrorDetail.cc'
--- src/ssl/ErrorDetail.cc	2016-09-11 14:59:06 +0000
+++ src/ssl/ErrorDetail.cc	2016-09-23 00:50:52 +0000
@@ -434,7 +434,7 @@
  */
 const char  *Ssl::ErrorDetail::subject() const
 {
-    if (broken_cert.get()) {
+    if (broken_cert) {
         static char tmpBuffer[256]; // A temporary buffer
         if (X509_NAME_oneline(X509_get_subject_name(broken_cert.get()), tmpBuffer, sizeof(tmpBuffer)))
             return tmpBuffer;

=== modified file 'src/ssl/PeekingPeerConnector.cc'
--- src/ssl/PeekingPeerConnector.cc	2016-09-22 14:21:12 +0000
+++ src/ssl/PeekingPeerConnector.cc	2016-09-23 00:50:19 +0000
@@ -222,7 +222,7 @@
 
     // remember the server certificate from the ErrorDetail object
     if (Ssl::ServerBump *serverBump = request->clientConnectionManager->serverBump()) {
-        if (!serverBump->serverCert.get()) {
+        if (!serverBump->serverCert) {
             // remember the server certificate from the ErrorDetail object
             if (error && error->detail && error->detail->peerCert())
                 serverBump->serverCert.resetAndLock(error->detail->peerCert());

=== modified file 'src/ssl/ServerBump.cc'
--- src/ssl/ServerBump.cc	2016-09-13 11:38:07 +0000
+++ src/ssl/ServerBump.cc	2016-09-23 01:07:41 +0000
@@ -54,7 +54,7 @@
 void
 Ssl::ServerBump::attachServerSSL(SSL *ssl)
 {
-    if (serverSSL.get())
+    if (serverSSL)
         return;
 
     serverSSL.resetAndLock(ssl);
@@ -63,7 +63,7 @@
 const Security::CertErrors *
 Ssl::ServerBump::sslErrors() const
 {
-    if (!serverSSL.get())
+    if (!serverSSL)
         return NULL;
 
     const Security::CertErrors *errs = static_cast<const Security::CertErrors*>(SSL_get_ex_data(serverSSL.get(), ssl_ex_index_ssl_errors));

=== modified file 'src/ssl/cert_validate_message.cc'
--- src/ssl/cert_validate_message.cc	2016-09-13 11:38:07 +0000
+++ src/ssl/cert_validate_message.cc	2016-09-23 00:50:03 +0000
@@ -53,7 +53,7 @@
             body +="\n";
             body = body + param_error_name + xitoa(i) + "=" + GetErrorName(err->element.code) + "\n";
             int errorCertPos = -1;
-            if (err->element.cert.get())
+            if (err->element.cert)
                 errorCertPos = sk_X509_find(peerCerts, err->element.cert.get());
             if (errorCertPos < 0) {
                 // assert this error ?

=== modified file 'src/ssl/crtd_message.cc'
--- src/ssl/crtd_message.cc	2016-01-01 00:12:18 +0000
+++ src/ssl/crtd_message.cc	2016-09-23 00:49:29 +0000
@@ -254,7 +254,7 @@
     std::string certsPart;
     if (!Ssl::writeCertAndPrivateKeyToMemory(certProperties.signWithX509, certProperties.signWithPkey, certsPart))
         throw std::runtime_error("Ssl::writeCertAndPrivateKeyToMemory()");
-    if (certProperties.mimicCert.get()) {
+    if (certProperties.mimicCert) {
         if (!Ssl::appendCertToMemory(certProperties.mimicCert, certsPart))
             throw std::runtime_error("Ssl::appendCertToMemory()");
     }

=== modified file 'src/ssl/gadgets.cc'
--- src/ssl/gadgets.cc	2016-07-08 06:24:33 +0000
+++ src/ssl/gadgets.cc	2016-09-23 00:46:51 +0000
@@ -25,7 +25,7 @@
     if (!rsa)
         return NULL;
 
-    if (!EVP_PKEY_assign_RSA(pkey.get(), (rsa.get())))
+    if (!EVP_PKEY_assign_RSA(pkey.get(), rsa.get()))
         return NULL;
 
     rsa.release();
@@ -65,7 +65,7 @@
     if (!bio)
         return false;
 
-    if (!PEM_write_bio_X509 (bio.get(), cert.get()))
+    if (!PEM_write_bio_X509(bio.get(), cert.get()))
         return false;
 
     if (!PEM_write_bio_PrivateKey(bio.get(), pkey.get(), NULL, NULL, 0, NULL, NULL))
@@ -89,7 +89,7 @@
     if (!bio)
         return false;
 
-    if (!PEM_write_bio_X509 (bio.get(), cert.get()))
+    if (!PEM_write_bio_X509(bio.get(), cert.get()))
         return false;
 
     char *ptr = NULL;
@@ -230,7 +230,7 @@
     static std::string certKey;
     certKey.clear();
     certKey.reserve(4096);
-    if (mimicCert.get()) {
+    if (mimicCert) {
         char buf[1024];
         certKey.append(X509_NAME_oneline(X509_get_subject_name(mimicCert.get()), buf, sizeof(buf)));
     }
@@ -273,15 +273,15 @@
 static bool
 mimicAuthorityKeyId(Security::CertPointer &cert, Security::CertPointer const &mimicCert, Security::CertPointer const &issuerCert)
 {
-    if (!mimicCert.get() || !issuerCert.get())
+    if (!mimicCert || !issuerCert)
         return false;
 
     Ssl::AUTHORITY_KEYID_Pointer akid((AUTHORITY_KEYID *)X509_get_ext_d2i(mimicCert.get(), NID_authority_key_identifier, nullptr, nullptr));
 
     bool addKeyId = false, addIssuer = false;
-    if (akid.get()) {
-        addKeyId = (akid.get()->keyid != nullptr);
-        addIssuer = (akid.get()->issuer && akid.get()->serial);
+    if (akid) {
+        addKeyId = (akid->keyid != nullptr);
+        addIssuer = (akid->issuer && akid->serial);
     }
 
     if (!addKeyId && !addIssuer)
@@ -299,18 +299,18 @@
 
     Ssl::X509_NAME_Pointer issuerName;
     Ssl::ASN1_INT_Pointer issuerSerial;
-    if (issuerKeyId.get() == nullptr || addIssuer) {
+    if (!issuerKeyId || addIssuer) {
         issuerName.reset(X509_NAME_dup(X509_get_issuer_name(issuerCert.get())));
         issuerSerial.reset(M_ASN1_INTEGER_dup(X509_get_serialNumber(issuerCert.get())));
     }
 
     Ssl::AUTHORITY_KEYID_Pointer theAuthKeyId(AUTHORITY_KEYID_new());
-    if (!theAuthKeyId.get())
+    if (!theAuthKeyId)
         return false;
-    theAuthKeyId.get()->keyid = issuerKeyId.release();
+    theAuthKeyId->keyid = issuerKeyId.release();
     if (issuerName && issuerSerial) {
         Ssl::GENERAL_NAME_STACK_Pointer genNames(sk_GENERAL_NAME_new_null());
-        if (genNames.get()) {
+        if (genNames) {
             if (GENERAL_NAME *aname = GENERAL_NAME_new()) {
                 sk_GENERAL_NAME_push(genNames.get(), aname);
                 aname->type = GEN_DIRNAME;
@@ -323,7 +323,7 @@
 
     // The Authority Key Identifier extension should include KeyId or/and both
     /// issuer name and issuer serial
-    if (!theAuthKeyId.get()->keyid && (!theAuthKeyId.get()->issuer || !theAuthKeyId.get()->serial))
+    if (!theAuthKeyId->keyid && (!theAuthKeyId->issuer || !theAuthKeyId->serial))
         return false;
 
     const X509V3_EXT_METHOD *method = X509V3_EXT_get_nid(NID_authority_key_identifier);
@@ -333,10 +333,10 @@
     unsigned char *ext_der = NULL;
     int ext_len = ASN1_item_i2d((ASN1_VALUE *)theAuthKeyId.get(), &ext_der, ASN1_ITEM_ptr(method->it));
     Ssl::ASN1_OCTET_STRING_Pointer extOct(M_ASN1_OCTET_STRING_new());
-    extOct.get()->data = ext_der;
-    extOct.get()->length = ext_len;
+    extOct->data = ext_der;
+    extOct->length = ext_len;
     Ssl::X509_EXTENSION_Pointer extAuthKeyId(X509_EXTENSION_create_by_NID(NULL, NID_authority_key_identifier, 0, extOct.get()));
-    if (!extAuthKeyId.get())
+    if (!extAuthKeyId)
         return false;
 
     extOct.release();
@@ -428,7 +428,7 @@
 {
     // not an Ssl::X509_NAME_Pointer because X509_REQ_get_subject_name()
     // returns a pointer to the existing subject name. Nothing to clean here.
-    if (properties.mimicCert.get()) {
+    if (properties.mimicCert) {
         // Leave subject empty if we cannot extract it from true cert.
         if (X509_NAME *name = X509_get_subject_name(properties.mimicCert.get())) {
             // X509_set_subject_name will call X509_dup for name
@@ -436,7 +436,7 @@
         }
     }
 
-    if (properties.setCommonName || !properties.mimicCert.get()) {
+    if (properties.setCommonName || !properties.mimicCert) {
         // In this case the CN of the certificate given by the user
         // Ignore errors: it is better to make a certificate with no CN
         // than to quit ssl-crtd helper because we cannot make a certificate.
@@ -450,8 +450,8 @@
     // Currently there is not any way in openssl tollkit to compare two ASN1_TIME
     // objects.
     ASN1_TIME *aTime = NULL;
-    if (!properties.setValidBefore && properties.mimicCert.get())
-        aTime = X509_get_notBefore(properties.mimicCert.get());
+    if (!properties.setValidBefore && properties.mimicCert)
+        aTime = X509_get_notBefore(properties.mimicCert);
     if (!aTime && properties.signWithX509.get())
         aTime = X509_get_notBefore(properties.signWithX509.get());
 
@@ -462,9 +462,9 @@
         return false;
 
     aTime = NULL;
-    if (!properties.setValidAfter && properties.mimicCert.get())
+    if (!properties.setValidAfter && properties.mimicCert)
         aTime = X509_get_notAfter(properties.mimicCert.get());
-    if (!aTime && properties.signWithX509.get())
+    if (!aTime && properties.signWithX509)
         aTime = X509_get_notAfter(properties.signWithX509.get());
     if (aTime) {
         if (!X509_set_notAfter(cert.get(), aTime))
@@ -473,7 +473,7 @@
         return false;
 
     // mimic the alias and possibly subjectAltName
-    if (properties.mimicCert.get()) {
+    if (properties.mimicCert) {
         unsigned char *alStr;
         int alLen;
         alStr = X509_alias_get0(properties.mimicCert.get(), &alLen);
@@ -508,7 +508,7 @@
 {
     Ssl::EVP_PKEY_Pointer pkey;
     // Use signing certificates private key as generated certificate private key
-    if (properties.signWithPkey.get())
+    if (properties.signWithPkey)
         pkey.resetAndLock(properties.signWithPkey.get());
     else // if not exist generate one
         pkey.resetWithoutLocking(Ssl::createSslPrivateKey());
@@ -532,7 +532,7 @@
 
     int ret = 0;
     // Set issuer name, from CA or our subject name for self signed cert
-    if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithX509.get())
+    if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithX509)
         ret = X509_set_issuer_name(cert.get(), X509_get_subject_name(properties.signWithX509.get()));
     else // Self signed certificate, set issuer to self
         ret = X509_set_issuer_name(cert.get(), X509_get_subject_name(cert.get()));
@@ -542,7 +542,7 @@
     const  EVP_MD *hash = properties.signHash ? properties.signHash : EVP_get_digestbyname(SQUID_SSL_SIGN_HASH_IF_NONE);
     assert(hash);
     /*Now sign the request */
-    if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithPkey.get())
+    if (properties.signAlgorithm != Ssl::algSignSelf && properties.signWithPkey)
         ret = X509_sign(cert.get(), properties.signWithPkey.get(), hash);
     else //else sign with self key (self signed request)
         ret = X509_sign(cert.get(), pkey.get(), hash);
@@ -614,7 +614,7 @@
     Security::CertPointer fakeCert;
 
     serial.reset(x509Pubkeydigest(properties.signWithX509));
-    if (!serial.get()) {
+    if (!serial) {
         serial.reset(BN_new());
         BN_zero(serial.get());
     }
@@ -742,7 +742,7 @@
 
     // For non self-signed certificates we have to check if the signing certificate changed
     if (properties.signAlgorithm != Ssl::algSignSelf) {
-        assert(properties.signWithX509.get());
+        assert(properties.signWithX509);
         if (X509_check_issued(properties.signWithX509.get(), cert) != X509_V_OK)
             return false;
     }

=== modified file 'src/ssl/support.cc'
--- src/ssl/support.cc	2016-09-22 14:21:12 +0000
+++ src/ssl/support.cc	2016-09-23 00:48:04 +0000
@@ -530,7 +530,7 @@
     port.secure.updateContextEecdh(ctx);
     port.secure.updateContextCa(ctx);
 
-    if (port.clientCA.get()) {
+    if (port.clientCA) {
         ERR_clear_error();
         if (STACK_OF(X509_NAME) *clientca = SSL_dup_CA_list(port.clientCA.get())) {
             SSL_CTX_set_client_CA_list(ctx.get(), clientca);



More information about the squid-dev mailing list