Ramkumar Ramachandra <artagnon_at_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
Received on 2010-10-12 11:26:05 CEST