>    1287 void SSL_free_comp_methods(void)
>    1288 {
>    1289     if (ssl_comp_methods == NULL)
>    1290             return;
>    1291     CRYPTO_w_lock(CRYPTO_LOCK_SSL);
>    1292     if (ssl_comp_methods != NULL)
>    1293     {
>    1294         sk_SSL_COMP_pop_free(ssl_comp_methods,CRYPTO_free);
>    1295         ssl_comp_methods = NULL;
>    1296     }
>    1297     CRYPTO_w_unlock(CRYPTO_LOCK_SSL);
>    1298 }

For POSIX threads, the result of reading a variable in one thread while it
might be modified in another thread is undefined. Line 1289 and 1290 should
be removed.

Alternatively, lines 1288, 1289, 1290, and 1297 should be removed. It's hard
to understand any reasonable scenario in which one thread might be calling
SSL_free_comp_methods while another thread is or might be accessing the
ssl_comp_methods variable or any of the registered compression methods.

But the code as posted is an unreasonable compromise between two reasonable
positions. It is "sort of" safe if called while another thread is or might
be accesing ssl_comp_methods.

If you think there is no way this code could fail, imagine a hypothetical
compiler/platform in which "if(ssl_comp_methods == NULL)" translates into an
operation through a fast caching shadow register like this:
shadow=ssl_comp_methods;
if(ssl_comp_methods==NULL)
{
 ssl_comp_methods=shadow; /* flush cache, writing back entries */
 /* whatever */
}

If another thread modified 'ssl_comp_methods' in between the read and the
cache flush, that modification would be lost. I, for one, don't like
needlessly making code potentially disastrous on future CPUs and compilers.
(To save what?!)

... the evils of premature optimization.

DS


______________________________________________________________________
OpenSSL Project                                 http://www.openssl.org
Development Mailing List                       openssl-dev@openssl.org
Automated List Manager                           [EMAIL PROTECTED]

Reply via email to