commit b9195402b41a4ec8e6141c31ff7b6c6561a29b3d
author David Benjamin <davidben@google.com> Fri Aug 05 10:51:43 2016 -0400
committer Adam Langley <agl@google.com> Fri Aug 05 19:00:33 2016 +0000
tree 977605a5d6a92d4331b92e34bd5e6256e9be8a4e
parent a9c3bf142e5425313a2197fcc84451b879d89237

Align SSL_SESSION_up_ref with OpenSSL.

Only X509_up_ref left (it's still waiting on a few external callers).

BUG=89

Change-Id: Ia2aec2bb0a944356cb1ce29f3b58a26bdb8a9977
Reviewed-on: https://boringssl-review.googlesource.com/9141
Commit-Queue: David Benjamin <davidben@google.com>
Reviewed-by: Adam Langley <agl@google.com>

include/openssl/ssl.h

diff --git a/include/openssl/ssl.h b/include/openssl/ssl.h
index fcceb89..02a0766 100644
--- a/include/openssl/ssl.h
+++ b/include/openssl/ssl.h
@@ -1533,9 +1533,9 @@
  * used outside the library. */
 OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_new(void);
 
-/* SSL_SESSION_up_ref, if |session| is not NULL, increments the reference count
- * of |session|. It then returns |session|. */
-OPENSSL_EXPORT SSL_SESSION *SSL_SESSION_up_ref(SSL_SESSION *session);
+/* SSL_SESSION_up_ref increments the reference count of |session| and returns
+ * one. */
+OPENSSL_EXPORT int SSL_SESSION_up_ref(SSL_SESSION *session);
 
 /* SSL_SESSION_free decrements the reference count of |session|. If it reaches
  * zero, all data referenced by |session| and |session| itself are released. */

ssl/handshake_client.c

diff --git a/ssl/handshake_client.c b/ssl/handshake_client.c
index 850b2bc..0b50505 100644
--- a/ssl/handshake_client.c
+++ b/ssl/handshake_client.c
@@ -508,7 +508,8 @@
 
         SSL_SESSION_free(ssl->s3->established_session);
         if (ssl->session != NULL) {
-          ssl->s3->established_session = SSL_SESSION_up_ref(ssl->session);
+          SSL_SESSION_up_ref(ssl->session);
+          ssl->s3->established_session = ssl->session;
         } else {
           /* We make a copy of the session in order to maintain the immutability
            * of the new established_session due to False Start. The caller may

ssl/handshake_server.c

diff --git a/ssl/handshake_server.c b/ssl/handshake_server.c
index c47ba67..a6d26d1 100644
--- a/ssl/handshake_server.c
+++ b/ssl/handshake_server.c
@@ -487,7 +487,8 @@
 
         SSL_SESSION_free(ssl->s3->established_session);
         if (ssl->session != NULL) {
-          ssl->s3->established_session = SSL_SESSION_up_ref(ssl->session);
+          SSL_SESSION_up_ref(ssl->session);
+          ssl->s3->established_session = ssl->session;
         } else {
           ssl->s3->established_session = ssl->s3->new_session;
           ssl->s3->established_session->not_resumable = 0;

ssl/ssl_lib.c

diff --git a/ssl/ssl_lib.c b/ssl/ssl_lib.c
index 64fca64..6a2dae8 100644
--- a/ssl/ssl_lib.c
+++ b/ssl/ssl_lib.c
@@ -2088,11 +2088,12 @@
     if (use_internal_cache) {
       SSL_CTX_add_session(ctx, ssl->s3->established_session);
     }
-    if (ctx->new_session_cb != NULL &&
-        !ctx->new_session_cb(ssl, SSL_SESSION_up_ref(
-            ssl->s3->established_session))) {
-      /* |new_session_cb|'s return value signals whether it took ownership. */
-      SSL_SESSION_free(ssl->s3->established_session);
+    if (ctx->new_session_cb != NULL) {
+      SSL_SESSION_up_ref(ssl->s3->established_session);
+      if (!ctx->new_session_cb(ssl, ssl->s3->established_session)) {
+        /* |new_session_cb|'s return value signals whether it took ownership. */
+        SSL_SESSION_free(ssl->s3->established_session);
+      }
     }
   }
 

ssl/ssl_session.c

diff --git a/ssl/ssl_session.c b/ssl/ssl_session.c
index 283639d..c3a4899 100644
--- a/ssl/ssl_session.c
+++ b/ssl/ssl_session.c
@@ -266,11 +266,9 @@
   return 0;
 }
 
-SSL_SESSION *SSL_SESSION_up_ref(SSL_SESSION *session) {
-  if (session != NULL) {
-    CRYPTO_refcount_inc(&session->references);
-  }
-  return session;
+int SSL_SESSION_up_ref(SSL_SESSION *session) {
+  CRYPTO_refcount_inc(&session->references);
+  return 1;
 }
 
 void SSL_SESSION_free(SSL_SESSION *session) {
@@ -384,8 +382,11 @@
 }
 
 SSL_SESSION *SSL_get1_session(SSL *ssl) {
-  /* variant of SSL_get_session: caller really gets something */
-  return SSL_SESSION_up_ref(SSL_get_session(ssl));
+  SSL_SESSION *ret = SSL_get_session(ssl);
+  if (ret != NULL) {
+    SSL_SESSION_up_ref(ret);
+  }
+  return ret;
 }
 
 int SSL_SESSION_get_ex_new_index(long argl, void *argp,

ssl/test/bssl_shim.cc

diff --git a/ssl/test/bssl_shim.cc b/ssl/test/bssl_shim.cc
index b2360de..61bdd7b 100644
--- a/ssl/test/bssl_shim.cc
+++ b/ssl/test/bssl_shim.cc
@@ -1421,8 +1421,8 @@
     } else if (config->async) {
       // The internal session cache is disabled, so install the session
       // manually.
-      GetTestState(ssl.get())->pending_session.reset(
-          SSL_SESSION_up_ref(session));
+      SSL_SESSION_up_ref(session);
+      GetTestState(ssl.get())->pending_session.reset(session);
     }
   }