svn commit: r1800836 - in /subversion/trunk/subversion: include/private/svn_dep_compat.h libsvn_fs/fs-loader.c libsvn_subr/utf.c

[ph...@apache.org]

Author: philip
Date: Tue Jul  4 22:37:52 2017
New Revision: 1800836

URL: http://svn.apache.org/viewvc?rev=1800836&view=rev
Log:
Remove some casts when using APR 2 and fix a "missing volatile" bug that
was uncovered.

* subversion/include/private/svn_dep_compat.h
  (svn_atomic_casptr, svn_atomic_xchgptr): New.

* subversion/libsvn_fs/fs-loader.c
  (struct fs_type_defn): Add missing volatile to vtable element, change
   type to void * to avoid cast (we lose no type safety since we never
   access the value as any other type).
  (get_library_vtable_direct): Remove explicit cast.

* subversion/libsvn_subr/utf.c
  (atomic_swap): Remove explicit cast.

Modified:
    subversion/trunk/subversion/include/private/svn_dep_compat.h
    subversion/trunk/subversion/libsvn_fs/fs-loader.c
    subversion/trunk/subversion/libsvn_subr/utf.c

Modified: subversion/trunk/subversion/include/private/svn_dep_compat.h
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/include/private/svn_dep_compat.h?rev=1800836&r1=1800835&r2=1800836&view=diff
==============================================================================
--- subversion/trunk/subversion/include/private/svn_dep_compat.h (original)
+++ subversion/trunk/subversion/include/private/svn_dep_compat.h Tue Jul  4 22:37:52 2017
@@ -115,6 +115,28 @@ extern "C" {
 #endif
 
 /**
+ * APR 1 has volatile qualifier bugs in some atomic prototypes that
+ * are fixed in APR 2:
+ *   https://issues.apache.org/bugzilla/show_bug.cgi?id=50731
+ * Subversion code should put the volatile qualifier in the correct
+ * place when declaring variables which means that casting at the call
+ * site is necessary when using APR 1.  No casts should be used with
+ * APR 2 as this allows the compiler to check that the variable has
+ * the correct volatile qualifier.
+ */
+#if APR_VERSION_AT_LEAST(2,0,0)
+#define svn_atomic_casptr(mem, with, cmp) \
+  apr_atomic_casptr((mem), (with), (cmp))
+#define svn_atomic_xchgptr(mem, val) \
+  apr_atomic_xchgptr((mem), (val))
+#else
+#define svn_atomic_casptr(mem, with, cmp) \
+  apr_atomic_casptr((void volatile **)(mem), (with), (cmp))
+#define svn_atomic_xchgptr(mem, val) \
+  apr_atomic_xchgptr((void volatile **)(mem), (val))
+#endif
+
+/**
  * Check at compile time if the Serf version is at least a certain
  * level.
  * @param major The major version component of the version checked

Modified: subversion/trunk/subversion/libsvn_fs/fs-loader.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_fs/fs-loader.c?rev=1800836&r1=1800835&r2=1800836&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_fs/fs-loader.c (original)
+++ subversion/trunk/subversion/libsvn_fs/fs-loader.c Tue Jul  4 22:37:52 2017
@@ -89,7 +89,7 @@ struct fs_type_defn {
   const char *fs_type;
   const char *fsap_name;
   fs_init_func_t initfunc;
-  fs_library_vtable_t *vtable;
+  void * volatile vtable; /* fs_library_vtable_t */
   struct fs_type_defn *next;
 };
 
@@ -190,7 +190,7 @@ get_library_vtable_direct(fs_library_vta
   const svn_version_t *fs_version;
 
   /* most times, we get lucky */
-  *vtable = apr_atomic_casptr((volatile void **)&fst->vtable, NULL, NULL);
+  *vtable = svn_atomic_casptr(&fst->vtable, NULL, NULL);
   if (*vtable)
     return SVN_NO_ERROR;
 
@@ -233,7 +233,7 @@ get_library_vtable_direct(fs_library_vta
                              fs_version->patch, fs_version->tag);
 
   /* the vtable will not change.  Remember it */
-  apr_atomic_casptr((volatile void **)&fst->vtable, *vtable, NULL);
+  svn_atomic_casptr(&fst->vtable, *vtable, NULL);
 
   return SVN_NO_ERROR;
 }

Modified: subversion/trunk/subversion/libsvn_subr/utf.c
URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/utf.c?rev=1800836&r1=1800835&r2=1800836&view=diff
==============================================================================
--- subversion/trunk/subversion/libsvn_subr/utf.c (original)
+++ subversion/trunk/subversion/libsvn_subr/utf.c Tue Jul  4 22:37:52 2017
@@ -190,9 +190,7 @@ static APR_INLINE void*
 atomic_swap(void * volatile * mem, void *new_value)
 {
 #if APR_HAS_THREADS
-  /* Cast is necessary because of APR bug:
-     https://issues.apache.org/bugzilla/show_bug.cgi?id=50731 */
-   return apr_atomic_xchgptr((volatile void **)mem, new_value);
+   return svn_atomic_xchgptr(mem, new_value);
 #else
    /* no threads - no sync. necessary */
    void *old_value = (void*)*mem;