Simplify `_crypt_extended_init_r`, and fix redundant initialization on Win32/Solaris

Looking at the history of this function, the original implementation had a bug where
it would return from the middle of the function without unlocking the mutex first.
The author attempted to fix this by incrementing the `initialized` flag atomically,
which is not necessary, since the section which modifies the flag is protected by a
mutex.

Coincidentally, at the same time that all this unnecessary 'atomic' machinery was
introduced, the code was also changed so that it didn't return without unlocking the
mutex. So it looks like the bug was fixed by accident.

It's not necessary to declare the flag as `volatile` either, since it is protected
by a mutex.

Further, the 'fixed' implementation was also wrong in another respect: on Windows
and Solaris, the `initialized` flag was not even declared as `static`!! So the
initialization of the static tables for S-boxes, P-boxes, etc. was repeated on
each call to `php_crypt`, completely defeating the purpose of this function.

diff --git a/configure.ac b/configure.ac
index 99e04e6c1d0be4eeb182dda4db6acd67f94997c0..df2c205f1973d1fca1dbfb86d1b5db3be932d8fc 100644 (file)
--- a/configure.ac
+++ b/configure.ac
@@ -643,13 +643,6 @@ if test "$ac_cv_func_getaddrinfo" = yes; then
   AC_DEFINE(HAVE_GETADDRINFO,1,[Define if you have the getaddrinfo function])
 fi
 
-dnl Check for the __sync_fetch_and_add builtin.
-AC_CACHE_CHECK([for __sync_fetch_and_add], ac_cv_func_sync_fetch_and_add,
-[AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[int x;__sync_fetch_and_add(&x,1);]])],[ac_cv_func_sync_fetch_and_add=yes],[ac_cv_func_sync_fetch_and_add=no])])
-if test "$ac_cv_func_sync_fetch_and_add" = yes; then
-  AC_DEFINE(HAVE_SYNC_FETCH_AND_ADD,1,[Define if you have the __sync_fetch_and_add function])
-fi
-
 AC_REPLACE_FUNCS(strlcat strlcpy explicit_bzero getopt)
 AC_FUNC_ALLOCA
 PHP_TIME_R_TYPE

diff --git a/ext/standard/config.m4 b/ext/standard/config.m4
index 171200d18d2d57935b76d964d541b5960c4be163..9f82ab1afa1a7d113ddfb7b9a69723e88e11f9d5 100644 (file)
--- a/ext/standard/config.m4
+++ b/ext/standard/config.m4
@@ -388,11 +388,6 @@ if test "$ac_cv_strptime_decl_fails" = "yes"; then
   AC_DEFINE([HAVE_STRPTIME_DECL_FAILS], 1, [whether strptime() declaration fails])
 fi
 
-dnl
-dnl Check for atomic operation API availability in Solaris
-dnl
-AC_CHECK_HEADERS([atomic.h])
-
 dnl
 dnl Check for arc4random on BSD systems
 dnl

diff --git a/ext/standard/php_crypt_r.c b/ext/standard/php_crypt_r.c
index 96e5905e5aeb5b2e9bbd7cc1b4e6ae99b33ebd77..432657cf47a371e8e7e337efcd2b22387d5ba288 100644 (file)
--- a/ext/standard/php_crypt_r.c
+++ b/ext/standard/php_crypt_r.c
@@ -39,11 +39,6 @@
 # include <Wincrypt.h>
 #endif
 
-#ifdef HAVE_ATOMIC_H /* Solaris 10 defines atomic API within */
-# include <atomic.h>
-#else
-# include <signal.h>
-#endif
 #include "php_crypt_r.h"
 #include "crypt_freesec.h"
 
@@ -76,29 +71,17 @@ void php_shutdown_crypt_r()
 
 void _crypt_extended_init_r(void)
 {
-#ifdef PHP_WIN32
-       LONG volatile initialized = 0;
-#elif defined(HAVE_ATOMIC_H) /* Solaris 10 defines atomic API within */
-       volatile unsigned int initialized = 0;
-#else
-       static volatile sig_atomic_t initialized = 0;
-#endif
+       static int initialized = 0;
 
 #ifdef ZTS
        tsrm_mutex_lock(php_crypt_extended_init_lock);
 #endif
 
        if (!initialized) {
-#ifdef PHP_WIN32
-               InterlockedIncrement(&initialized);
-#elif defined(HAVE_SYNC_FETCH_AND_ADD)
-               __sync_fetch_and_add(&initialized, 1);
-#elif defined(HAVE_ATOMIC_H) /* Solaris 10 defines atomic API within */
-               membar_producer();
-               atomic_add_int(&initialized, 1);
-#endif
+               initialized = 1;
                _crypt_extended_init();
        }
+
 #ifdef ZTS
        tsrm_mutex_unlock(php_crypt_extended_init_lock);
 #endif