Ivan Zhakov <i...@visualsvn.com> writes: > I was thinking about similar change, but as far I remember it's not > safe to use svn_atomic__init_once() in error.c, because > svn_atomic__init_once() uses svn_error_*() API. This may lead circular > initialization of svn_error_*() tracing infrastructure.
That does need to be considered. The atomic init callback in this case is written to never return an error so the only errors to consider are those raised by svn_atomic__init_once itself. Any such error would be a failure of svn_atomic_cas. Possible causes: the svn_atomic_t passed in is something other than 0/1/2 or the svn_atomic_t was read-only. Anything else? If I initialize: static volatile svn_atomic_t init_status = 10; the client does go into an infinite loop when attempting to raise an error but that doesn't have anything to do with this patch, the same happens with any call to svn_atomic__init_once. If I initialize static volatile svn_atomic_t init_status = 2; the client recurses until it SEGVs when svn_atomic__init_once tries to raise an error, that is caused by this patch. The error that svn_atomic__init_once is trying to raise would be discarded by svn_error__locate. A memory overwrite bug could cause the svn_atomic_t used by svn_error__locate to become 2 but equally a memory overwrite bug could set any other svn_atomic_t to 10 and cause an infinite loop. We could add a "no errors" flag to svn_atomic__init_once to avoid the recursion/SEGV caused when the svn_atomic_t is 2 but trying to handle memory overwrite is never going to be reliable. Overall I don't think the circular problem is a problem in practice. -- Philip Martin | Subversion Committer WANdisco // *Non-Stop Data*
