[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: A proposed solution for svn admin directory names

From: Joseph Galbraith <galb_at_vandyke.com>
Date: 2005-06-29 19:26:32 CEST

Garrett Rooney wrote:
> Joseph Galbraith wrote:
>
>> C. Michael Pilato wrote:
>>
>>> Philip Martin <philip@codematters.co.uk> writes:
>>>
>>>
>>>> Ben Collins-Sussman <sussman@collab.net> writes:
>>>>
>>>>
>>>>> On Jun 29, 2005, at 10:51 AM, Jonathan Malek wrote:
>>>>>
>>>>>> #define SVN_WC_ADM_DIR_NAME ( ( getenv( "SVN_ADM_DIR") ) ? ( getenv(
>>>>>> "SVN_ADM_DIR") ) : (".svn") )
>>>>
>>>>
>>>>
>>>>> IANTSP -- "I am not the subversion project". Do any other developers
>>>>> have opinions about this?
>>>>
>>>>
>>>>
>>>> Calling getenv() that often is horrible.
>>>
>>>
>>>
>>>
>>> Well ... that answers one question in my previous email. :-)
>>
>>
>>
>> Well, maybe this is better, or perhaps more horrible (excuse
>> the C++ism and pseudo-hungarianisms:
>>
>> char* getadmdir()
>> {
>> static bool bHaveDir = false;
>> static char* szAdmDir = 0;
>> if ( ! bHaveDir )
>> {
>> bHaveDir = true;
>> szAdmDir = ::getevn("SVN_ADM_DIR");
>> if ( szAdmDir == 0 )
>> szAdmDir = ".svn";
>> }
>>
>> return szAdmDir;
>> }
>>
>> #define SVN_WC_ADM_DIR_NAME getadmdir()
>
> Static variables like that are not especially thread safe.

Ahh... I think my second code snippet address this problem
as it does away with the bool and only uses the szAdmDir...
I'll quote it here for clarity:

char* getadmdir()
{
   static char* szAdmDir = 0;
   if ( szAdmDir == 0 )
   {
     szAdmDir = ::getenv("SVN_ADM_DIR");
     if ( szAdmDir == 0 )
        szAdmDir = ".svn";
   }

   return szAdmDir;
}

#define SVN_WC_ADM_DIR_NAME getadmdir()

In this case, there is no observable instance
where szAdmDir isn't either 0 or valid. In
all cases where it is 0, the thread sets it
to the correct thing (if another thread beat
it to the punch it might be resetting it, almost
certainly to an identical value, but it will
still be valid at all observable points.)

Of course, in the n threaded worst case
scenario, ::genenv() could be called n
times.

> And you're
> still depending on an environment variable, which seems like a rather
> fragile way to do configuration options like this.

Ahh... well, I wasn't addressing that issue... but, it can probably
be addressed similarly if you'd like to.

In other words, a static variable that is either 0 or a valid
value and code to read a config file that can harmlessly
execute on multiple threads simultaneously. (i.e., is stack
or heap based, doesn't modify globals itself, and allows the
file to be read by multiple readers.) It might be worth
introducing some synchronzation though to avoid reading
the file multiple times... or if there is some point
where we know we are single threaded (an initialization
routine?) we could trigger the code there to ensure initialization
is only preformed on one thread at a time.

Personally, I think the environment variable is a good
solution in that it solves the problem simply.

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 19:22:20 2005

This is an archived mail posted to the Subversion Dev mailing list.