After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 775292 - dtls: Set openssl's threadid the 1.0.x way
dtls: Set openssl's threadid the 1.0.x way
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Windows
: Normal normal
: 1.10.3
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks: 775281
 
 
Reported: 2016-11-29 00:06 UTC by Scott D Phillips
Modified: 2016-12-07 09:24 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
[PATCH gst-plugins-bad] dtls: Set openssl's threadid the 1.0.x way (1.66 KB, patch)
2016-11-29 00:08 UTC, Scott D Phillips
none Details | Review
[PATCH gst-plugins-bad v2] dtls: Set openssl's threadid the 1.0.x way (1.65 KB, patch)
2016-11-29 17:35 UTC, Scott D Phillips
committed Details | Review

Description Scott D Phillips 2016-11-29 00:06:57 UTC
In openssl 0.9.x the thread_id had to be an unsigned long. In 1.0.x the
thread_id could also be a pointer. In 1.1.x the whole thing was thrown
away.

With msvc compiling for x64, unsigned long is 4 bytes and can't hold a
pointer, causing a warning. So add logic to set the threadid the 1.0.x
way, which avoids the warning.
Comment 1 Scott D Phillips 2016-11-29 00:08:04 UTC
Created attachment 340952 [details] [review]
[PATCH gst-plugins-bad] dtls: Set openssl's threadid the 1.0.x way
Comment 2 Scott D Phillips 2016-11-29 00:16:10 UTC
If we don't want the #ifdef soup, we can also just have just the 0.9.x way and cast from pointer to intptr_t to unsigned long, squelching the warning. That'll work as long as all GThread*s are unique in their low 32-bits.
Comment 3 Sebastian Dröge (slomo) 2016-11-29 11:16:17 UTC
Review of attachment 340952 [details] [review]:

::: ext/dtls/gstdtlsagent.c
@@ +150,3 @@
       CRYPTO_set_id_callback (ssl_thread_id_function);
+#else /* OPENSSL_VERSION_NUMBER >= 0x10000000L */
+      CRYPTO_THREADID_set_callback (ssl_thread_id_function);

In 1.1, the first function is hard-deprecated (#if OPENSSL_API_COMPAT < 0x10000000L) and the second is #defined to a no-op. Let's add another #ifdef to just not call any of these for 1.1?
Comment 4 Scott D Phillips 2016-11-29 17:35:58 UTC
Created attachment 340990 [details] [review]
[PATCH gst-plugins-bad v2] dtls: Set openssl's threadid the 1.0.x way

(In reply to Sebastian Dröge (slomo) from comment #3)
> Review of attachment 340952 [details] [review] [review]:
> 
> ::: ext/dtls/gstdtlsagent.c
> @@ +150,3 @@
>        CRYPTO_set_id_callback (ssl_thread_id_function);
> +#else /* OPENSSL_VERSION_NUMBER >= 0x10000000L */
> +      CRYPTO_THREADID_set_callback (ssl_thread_id_function);
> 
> In 1.1, the first function is hard-deprecated (#if OPENSSL_API_COMPAT <
> 0x10000000L) and the second is #defined to a no-op. Let's add another #ifdef
> to just not call any of these for 1.1?

The logic to ifdef out all of this in 1.1 and later was already there but was getting a bit messy with my previous change. Looking at it again I realized that dtls already requires 1.0.1 or later, so a cleaner change is to just dump the 0.9.x way altogether.
Comment 5 Sebastian Dröge (slomo) 2016-12-05 09:27:19 UTC
commit 1a43d5735972c525b8cc69d46b392e903bb4a0aa
Author: Scott D Phillips <scott.d.phillips@intel.com>
Date:   Mon Nov 28 15:57:33 2016 -0800

    dtls: Set openssl's threadid the 1.0.x way
    
    For pre-1.1.x openssl, a callback to set the thread id needs to be
    provided to openssl. In 0.9.x the thread id was an unsigned long. In
    1.0.x it was expanded to be able to hold a void*. Here we change to use
    the 1.0.x API so that the thread id can always hold a GThread*, even on
    platforms like msvc x64 where unsigned long is only 32 bits.
    
    All of this is still #ifdef'd out of existence when building with
    openssl 1.1.x or later which changed the thread API again, and does not
    need a thread id callback.
    
    https://bugzilla.gnome.org/show_bug.cgi?id=775292
Comment 6 Sebastian Dröge (slomo) 2016-12-05 09:27:57 UTC
The DTLS plugin is only built if openssl >= 1.0.1 is found anyway.