Re: [PATCH 2/3] run-command: factor out running_main_thread function

2013-04-15 Thread Jonathan Nieder
Jeff King wrote:

 --- a/run-command.c
 +++ b/run-command.c
 @@ -603,7 +603,7 @@ static NORETURN void die_async(const char *err, va_list 
 params)
  {
   vreportf(fatal: , err, params);
  
 - if (!pthread_equal(main_thread, pthread_self())) {
 + if (!running_main_thread()) {
   struct async *async = pthread_getspecific(async_key);
   if (async-proc_in = 0)
   close(async-proc_in);
 @@ -614,6 +614,19 @@ static NORETURN void die_async(const char *err, va_list 
 params)
  
   exit(128);
  }
 +
 +int running_main_thread(void)
 +{
 + return pthread_equal(main_thread, pthread_self());
 +}

Would it make sense to do something like

return !main_thread_set || pthread_equal(...);

in case someone tries to call this before the first start_async call?
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


Re: [PATCH 2/3] run-command: factor out running_main_thread function

2013-04-15 Thread Jeff King
On Mon, Apr 15, 2013 at 06:45:04PM -0700, Jonathan Nieder wrote:

 Jeff King wrote:
 
  --- a/run-command.c
  +++ b/run-command.c
  @@ -603,7 +603,7 @@ static NORETURN void die_async(const char *err, va_list 
  params)
   {
  vreportf(fatal: , err, params);
   
  -   if (!pthread_equal(main_thread, pthread_self())) {
  +   if (!running_main_thread()) {
  struct async *async = pthread_getspecific(async_key);
  if (async-proc_in = 0)
  close(async-proc_in);
  @@ -614,6 +614,19 @@ static NORETURN void die_async(const char *err, 
  va_list params)
   
  exit(128);
   }
  +
  +int running_main_thread(void)
  +{
  +   return pthread_equal(main_thread, pthread_self());
  +}
 
 Would it make sense to do something like
 
   return !main_thread_set || pthread_equal(...);
 
 in case someone tries to call this before the first start_async call?

I considered that, but assumed that pthread_equal would always return
false against the zeroed out static main_thread. Mostly because we
appeared not to be bothering with such a check already.  But actually,
in the existing code, it is only checked from die_async, which is only
put into place when we _do_ start a thread, so we would never hit this
code path without main_thread being set.

So yes, a check probably would make sense. However, I think I'm
convinced that we should drop this in favor of using thread-local
storage for the dying counter. So this patch can just go away.

Thanks for reviewing this version, though.

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