Duane Griffin wrote:
> On Wed, 2005-06-29 at 18:26, Joseph Galbraith wrote:
>
>>char* getadmdir()
>>{
>> static char* szAdmDir = 0;
>> if ( szAdmDir == 0 )
>> {
>> szAdmDir = ::getenv("SVN_ADM_DIR");
>> if ( szAdmDir == 0 )
>> szAdmDir = ".svn";
>> }
>>
>> return szAdmDir;
>>}
>
>
> Aside from the possibility of non-atomic pointer assignment, there is
> another failure case where the function returns NULL:
>
> Consider the situation with two threads A and B where SVN_ADM_DIR is not
> set. A and B both enter the function and the first if block. Thread A
> continues into the second if block and does the assignment. Now thread B
> executes the getenv line. Thread A executes the return statement,
> returning the current global value of szAdmDir, which has been set by
> thread B to NULL. Bang.
>
> It really isn't possible to do this sort of thing in a thread-safe
> manner without synchronization.
Well, given that the non-atomic pointer assignment does seem to kill
it in the portable sense, it probably isn't worthwhile continuing this
discussion too much further. (And the objection to the use of
environment variables for config in general-- though there is precednet
in the SVN_SSH environment variable.)
But, wouldn't the introduction of a temporary resolve the
problem you point out above?
char* szEnv = ::getenv("SVN_ADM_DIR");
if ( szEnv == 0 )
szAdmDir = ".svn"
else
szAdmDir = szEnv;
Now szAdmDir is written exactly one time (in each thread), always
with a valid value.
Thanks for the careful analysis, by the way. I'll keep an eye
out for that particular mistaken pattern in my own code.
Thanks,
Joseph
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Wed Jun 29 20:48:09 2005