static-scoped functions highlight unchecked pthread_create problem

Jeff Darcy jdarcy at redhat.com
Tue Oct 5 16:58:53 UTC 2010


On 10/05/2010 04:03 AM, Jim Meyering wrote:
> Now that I've static-scoped another batch of functions,
> I'm seeing this new warning:
> 
>   proxy.c: In function 'repl_worker':
>   proxy.c:796: warning: no return statement in function returning non-void [-Wreturn-type]
> 
> repl_worker is returning void* because that's the signature
> it must have, since it's used here:
> 
>     pthread_create(&tid,NULL,repl_worker,NULL);
> 
> pthread_create itself uses the return value from that start_routine
> function if ever it returns.  Since repl_worker never returned,
> of course, it didn't have a return statement.  However, it made
> system calls that could fail.  In this case, it must return,
> to indicate their failure.  If pthread_create is failing because there
> is too little free memory or no free process table slot, then it would
> obviously be better not to infloop here.

I would have NAKed if I'd been given a chance, since the removal of the
"item" thread parameter was causing iwhd to segfault any time
replication is attempted.  I'll roll the fix for the "fix" into the
coming-soon new version of my replication-test patch.

> 
> Here's a fix:
> 
>>From aeb23797cf2e1e8ce6d57ec341235d48abb05e2f Mon Sep 17 00:00:00 2001
> From: Jim Meyering <meyering at redhat.com>
> Date: Tue, 5 Oct 2010 08:02:00 +0200
> Subject: [PATCH] don't ignore failed thread creation
> 
> * proxy.c (xpthread_create): New macro.
> (repl_worker): Use it to diagnose thread creation failure and to
> propagate any failure to caller.
> ---
>  proxy.c |   25 ++++++++++++++++---------
>  1 files changed, 16 insertions(+), 9 deletions(-)
> 
> diff --git a/proxy.c b/proxy.c
> index bf4c165..fa25e54 100644
> --- a/proxy.c
> +++ b/proxy.c
> @@ -744,6 +744,16 @@ repl_worker_bcreate (repl_item *item)
>  	DPRINTF("%s returning\n",__func__);
>  }
> 
> +/* Use this to diagnose failed thread creation.  */
> +#define xpthread_create(thread, start_routine, msg)			\
> +  do {									\
> +    int err = pthread_create (thread, NULL, start_routine, NULL);	\
> +    if (err) {								\
> +      error (0, err, msg);						\
> +      return NULL;							\
> +    }									\
> +  } while (0)
> +
>  static void *
>  repl_worker (void *notused ATTRIBUTE_UNUSED)
>  {
> @@ -764,15 +774,12 @@ repl_worker (void *notused ATTRIBUTE_UNUSED)
>  		switch (item->type) {
>  		case REPL_PUT:
>  			if (pipe(item->pipes) >= 0) {
> -				if (proxy_host) {
> -					pthread_create(&prod,NULL,
> -						proxy_repl_prod,item);
> -				}
> -				else {
> -					pthread_create(&prod,NULL,
> -						proxy_repl_prod_fs,item);
> -				}
> -				pthread_create(&cons,NULL,proxy_repl_cons,item);
> +				xpthread_create(&prod, (proxy_host
> +							? proxy_repl_prod
> +							: proxy_repl_prod_fs),
> +					      "failed to start producer thread");
> +				xpthread_create(&cons,proxy_repl_cons,
> +					      "failed to start consumer thread");
>  				pthread_join(prod,NULL);
>  				pthread_join(cons,NULL);
>  			}
> --
> 1.7.3.1.45.g9855b


More information about the iwhd-devel mailing list