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