[RFC] reverting ABI change in sslContext introduced in 4.1.2

--liOOAslEiF7prFVr
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

Hello,

while preparing Debian packages for omniORB4 4.1.2, we noticed the
following change in include/omniORB4/sslContext.h:

class sslContext {
private:
// ...
  SSL_CTX*          pd_ctx;
  omni_tracedmutex* pd_locks;
  CORBA::Boolean    pd_ssl_owner;   <-- this attribute was added
};

According to [1], to make a binary compatible change in C++, you can't:
  "add new data members to a class or change order of data members in a
   class (doesn't apply to static ones)"

so this change breaks the ABI. Since the SONAME of libomnisslTP4 was not
changed it's safer to rebuild all binaries depending on the sslContext
class (e.g. the Python module _omnisslTPmodule.so.3). Of course I'd
like to avoid that, and would prefer not to bump the SONAME, since that
would introduce a change between upstream and Debian versions.

The attached patch is another approach that does not break the ABI.
Could you please review it? It's supposed to be equivalent to the other
one.

Thanks,
Regards,

Thomas

[1] http://techbase.kde.org/index.php?title=Policies/Binary_Compatibility_Issues_With_C%2B%2B#The_Do.27s_and_Don.27ts

--liOOAslEiF7prFVr
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="omniorb4_revert_sslcontext_abi_change.diff"

Index: include/omniORB4/sslContext.h
===================================================================
--- include/omniORB4/sslContext.h	(revision 152)
+++ include/omniORB4/sslContext.h	(working copy)
@@ -159,7 +159,6 @@
   const char* 	    pd_password;
   SSL_CTX*    	    pd_ctx;
   omni_tracedmutex* pd_locks;
-  CORBA::Boolean    pd_ssl_owner;
 };
 
 #undef _core_attr
Index: src/lib/omniORB/orbcore/ssl/sslContext.cc
===================================================================
--- src/lib/omniORB/orbcore/ssl/sslContext.cc	(revision 152)
+++ src/lib/omniORB/orbcore/ssl/sslContext.cc	(working copy)
@@ -118,19 +118,42 @@
 
 sslContext* sslContext::singleton = 0;
 
+static sslContext* ssl_owner = 0;
 
+static bool become_ssl_owner(sslContext* context) {
+  bool become_owner = false;
+
+  if (ssl_owner == 0 && CRYPTO_get_locking_callback() == 0) {
+    ssl_owner = context;
+    become_owner = true;
+  }
+
+  return become_owner;
+}
+
+static bool was_ssl_owner(sslContext* context) {
+  bool was_owner = false;
+
+  if (ssl_owner != 0 && ssl_owner == context) {
+    was_owner = true;
+    ssl_owner = 0;
+  }
+
+  return was_owner;
+}
+
 /////////////////////////////////////////////////////////////////////////
 sslContext::sslContext(const char* cafile,
 		       const char* keyfile,
 		       const char* password) :
   pd_cafile(cafile), pd_keyfile(keyfile), pd_password(password), pd_ctx(0),
-  pd_locks(0), pd_ssl_owner(0) {}
+  pd_locks(0) {}
 
 
 /////////////////////////////////////////////////////////////////////////
 sslContext::sslContext() :
   pd_cafile(0), pd_keyfile(0), pd_password(0), pd_ctx(0),
-  pd_locks(0), pd_ssl_owner(0) {
+  pd_locks(0) {
 }
 
 /////////////////////////////////////////////////////////////////////////
@@ -140,7 +163,7 @@
   if (pd_ctx) return;
 
   // Assume we own the ssl if no locking callback yet.
-  pd_ssl_owner = CRYPTO_get_locking_callback() == 0;
+  bool pd_ssl_owner = become_ssl_owner(this);
 
   if (pd_ssl_owner) {
     SSL_library_init();
@@ -172,7 +195,7 @@
   if (pd_ctx) {
     SSL_CTX_free(pd_ctx);
   }
-  if (pd_ssl_owner)
+  if (was_ssl_owner(this))
     thread_cleanup();
 }
 

--liOOAslEiF7prFVr
Content-Type: text/plain; charset="us-ascii"
MIME-Version: 1.0
Content-Transfer-Encoding: 7bit
Content-Disposition: inline

_______________________________________________
omniORB-dev mailing list
[email protected]
http://www.omniorb-support.com/mailman/listinfo/omniorb-dev

--liOOAslEiF7prFVr--