[webkit-changes] cvs commit: JavaScriptCore/pcre pcre.h pcre_compile.c pcre_exec.c

[Maciej Stachowiak]

On Jan 6, 2006, at 4:31 PM, Geoffrey Garen wrote:

> We originally switched to the _r variants to prevent two localtime 
> () calls in the same stack from stomping each other.  This method  
> accomplishes that just as well.
>
> For thread safety, we're counting on OS's to use thread-local- 
> storage for globals.

Nothing in this change will protect you from two threads that call  
localtime at once. What does "thread-local storage for globals" have  
to do with it? localtime is not required to use thread-local storage  
for the struct tm it gives you a pointer to, and probably doesn't on  
most platforms. This should be fixed.

Regards,
Maciej


>
> Geoff
>
> On Jan 6, 2006, at 3:51 PM, George Staikos wrote:
>
>> On Friday 06 January 2006 17:43, David wrote:
>>>   -  tm t;
>>>   -  utc ? gmtime_r(&tv, &t) : localtime_r(&tv, &t);
>>>   +  tm t = *(utc ? gmtime(&tv) : localtime(&tv));
>>>      // We had an out of range year. Restore the year (plus/minus  
>>> offset
>>>      // found by calculating tm_year) and fix the week day  
>>> calculation.
>>>      if (realYearOffset != 0) {
>>>   @@ -769,8 +779,7 @@
>>>    JSValue *DateObjectImp::callAsFunction(ExecState * /*exec*/,  
>>> JSObject *
>>> /*thisObj*/, const List &/*args*/) {
>>>        time_t t = time(0);
>>>   -    tm ts;
>>>   -    localtime_r(&t, &ts);
>>>   +    tm ts = *localtime(&t);
>>>        return jsString(formatDate(ts) + " " + formatTime(ts));
>>>    }
>>>
>>>   @@ -899,8 +908,7 @@
>>>        // produce incorrect results, but there's no other option  
>>> when
>>> calling localtime_r(). if (!utc) {
>>>            time_t tval = mktime(t) + (time_t)((ms + yearOffset) /  
>>> 1000);
>>>   -        struct tm t3;
>>>   -        localtime_r(&tval, &t3);
>>>   +        tm t3 = *localtime(&tval);
>>>            t->tm_isdst = t3.tm_isdst;
>>>        }
>>
>>    These look quite questionable to me.  Is threadsafety not a  
>> goal anymore?
>>
>> -- 
>> George Staikos
>> KDE Developer				http://www.kde.org/
>> Staikos Computing Services Inc.		http://www.staikos.net/
>> _______________________________________________
>> webkit-changes mailing list
>> webkit-changes at opendarwin.org
>> http://www.opendarwin.org/mailman/listinfo/webkit-changes
>
> _______________________________________________
> webkit-changes mailing list
> webkit-changes at opendarwin.org
> http://www.opendarwin.org/mailman/listinfo/webkit-changes