Re: [CVS] OpenSSL: OpenSSL_1_0_1-stable: openssl/crypto/ cryptlib.c
On Mon, Sep 17, 2012 at 6:24 PM, Bodo Moeller b...@openssl.org wrote: OpenSSL CVS Repository http://cvs.openssl.org/ Server: cvs.openssl.org Name: Bodo Moeller Root: /v/openssl/cvs Email: b...@openssl.org Module: openssl Date: 17-Sep-2012 19:24:44 Branch: OpenSSL_1_0_1-stable Handle: 2012091717244400 Modified files: (Branch: OpenSSL_1_0_1-stable) openssl/crypto cryptlib.c Log: Fix warning. Submitted by: Chromium Authors Summary: RevisionChanges Path 1.75.2.5.2.8+1 -1 openssl/crypto/cryptlib.c patch -p0 '@@ .' Index: openssl/crypto/cryptlib.c $ cvs diff -u -r1.75.2.5.2.7 -r1.75.2.5.2.8 cryptlib.c --- openssl/crypto/cryptlib.c 8 Jun 2012 09:18:32 - 1.75.2.5.2.7 +++ openssl/crypto/cryptlib.c 17 Sep 2012 17:24:44 - 1.75.2.5.2.8 @@ -504,7 +504,7 @@ CRYPTO_THREADID_set_numeric(id, (unsigned long)find_thread(NULL)); #else /* For everything else, default to using the address of 'errno' */ - CRYPTO_THREADID_set_pointer(id, errno); + CRYPTO_THREADID_set_pointer(id, (void*)errno); This seems like a terrible idea. On some platforms... #define errno (* __error()) Clearly you do _not_ want to be writing to a function pointer... If you get a warning from passing errno as a void *, then you should probably pay attention to it, not make it go away... #endif } @@ . __ OpenSSL Project http://www.openssl.org CVS Repository Commit List openssl-...@openssl.org Automated List Manager majord...@openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [CVS] OpenSSL: OpenSSL_1_0_1-stable: openssl/crypto/ cryptlib.c
On Tue, Sep 18, 2012 at 9:47 AM, Ben Laurie b...@links.org wrote: On Mon, Sep 17, 2012 at 6:24 PM, Bodo Moeller b...@openssl.org wrote: OpenSSL CVS Repository http://cvs.openssl.org/ Server: cvs.openssl.org Name: Bodo Moeller Root: /v/openssl/cvs Email: b...@openssl.org Module: openssl Date: 17-Sep-2012 19:24:44 Branch: OpenSSL_1_0_1-stable Handle: 2012091717244400 Modified files: (Branch: OpenSSL_1_0_1-stable) openssl/crypto cryptlib.c Log: Fix warning. Submitted by: Chromium Authors Summary: RevisionChanges Path 1.75.2.5.2.8+1 -1 openssl/crypto/cryptlib.c patch -p0 '@@ .' Index: openssl/crypto/cryptlib.c $ cvs diff -u -r1.75.2.5.2.7 -r1.75.2.5.2.8 cryptlib.c --- openssl/crypto/cryptlib.c 8 Jun 2012 09:18:32 - 1.75.2.5.2.7 +++ openssl/crypto/cryptlib.c 17 Sep 2012 17:24:44 - 1.75.2.5.2.8 @@ -504,7 +504,7 @@ CRYPTO_THREADID_set_numeric(id, (unsigned long)find_thread(NULL)); #else /* For everything else, default to using the address of 'errno' */ - CRYPTO_THREADID_set_pointer(id, errno); + CRYPTO_THREADID_set_pointer(id, (void*)errno); This seems like a terrible idea. On some platforms... #define errno (* __error()) Clearly you do _not_ want to be writing to a function pointer... If you get a warning from passing errno as a void *, then you should probably pay attention to it, not make it go away... Doh. I see it doesn't write to it. Nevertheless, seems like a bad piece of code - its assuming errno is thread local, right? #endif } @@ . __ OpenSSL Project http://www.openssl.org CVS Repository Commit List openssl-...@openssl.org Automated List Manager majord...@openssl.org __ OpenSSL Project http://www.openssl.org Development Mailing List openssl-dev@openssl.org Automated List Manager majord...@openssl.org
Re: [CVS] OpenSSL: OpenSSL_1_0_1-stable: openssl/crypto/ cryptlib.c
Doh. I see it doesn't write to it. Nevertheless, seems like a bad piece of code - its assuming errno is thread local, right? This code uses the address of errno as a default thread ID for OpenSSL purposes. This works precisely because you typically have something like #define errno (*__error()) where the given function returns a thread-specific pointer, although the relevant standards don't guarantee that. (Obviously different thread's instances of errno aren't allowed to interfere with each other, but the standards don't say how that's achieved.) We don't promise that this works everywhere, we just offer this as a default.
