Ramkumar Ramachandra <artag...@gmail.com> writes: > Right, got it. Can I get a +1 for the following patch?
If we are going to use the APR atomic interface then the two reads should use apr_atomic_read32. It would be better to use svn_atomic__init_once. It's a clear indication that we are doing once only initialisation, so we don't need all the comments, and it avoids any problems related to the size of apr_fileperms_t. Also if enhancements are required (more memory barriers say) then svn_atomic__init_once is the place to do it. > > [[[ > * subversion/libsvn_subr/io.c > > (get_default_file_perms): The 32-bit integer `default_perms` is only > really atomic (and hence thread-safe) on x86 and x64 where compilers > handle the proper alignment for static variables. Handle it in the > more general case using `apr_atomic_set32`. > > Suggested by: Bert > ]]] > > Index: subversion/libsvn_subr/io.c > =================================================================== > --- subversion/libsvn_subr/io.c (revision 1005706) > +++ subversion/libsvn_subr/io.c (working copy) > @@ -1303,7 +1303,7 @@ get_default_file_perms(apr_fileperms_t *perms, apr > SVN_ERR(svn_io_file_close(fd, scratch_pool)); > > *perms = finfo.protection; > - default_perms = finfo.protection; > + apr_atomic_set32(&default_perms, finfo.protection); > } > else > *perms = default_perms; > -- Philip
