Re: [PATCH] MSVC: Windows-native implementation for subset of Pthreads API

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



2009/11/4 Nicolas Pitre <nico@xxxxxxxxxxx>:
> On Wed, 4 Nov 2009, Andrzej K. Haczewski wrote:
>
>> +     NO_STATIC_PTHREADS_INIT = YesPlease
>
> Let's not go that route please.  If Windows can't get away without
> runtime initializations then let's use them on all platforms.  There is
> no gain in exploding the code path combinations here wrt testing
> coverage.
>

I don't like that approach either, but I was frighten of Junio being
anal about static inits ;).

Let's make it clear: has anyone have any objections that I add
explicit initialization of mutexes and condition variables for POSIX
also?

>> +static THREAD_RET_TYPE threaded_find_deltas(void *arg)
>
> Why can't you just cast the function pointer in your pthread_create
> wrapper instead?  No one cares about the returned value anyway.

Because of calling convention - I'd have to cast cdecl function as
stdcall function, which would change the function call clean up (in
cdecl caller is responsible for unwinding stack, stdcall callee; the
effect - double stack unwinding).

>> @@ -2327,6 +2354,8 @@ int cmd_pack_objects(int argc, const char **argv, const char *prefix)
>>  #ifdef THREADED_DELTA_SEARCH
>>       if (!delta_search_threads)      /* --threads=0 means autodetect */
>>               delta_search_threads = online_cpus();
>> +
>> +     init_threaded_delta_search();
>
> What about doing this at the beginning of ll_find_deltas() instead?
> And similarly for cleanup_threaded_delta_search(): call it right before
> leaving ll_find_deltas().  This way thread issues would remain more
> localized.  In fact I'd move the whole thing above in ll_find_deltas()
> as well (separately from this patch though).

Sounds sensible, but I'd wait for the NO_STATIC_PTHREADS_INIT verdict.

--
Andrzej
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]