"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
Re: double lock in tog(1)
To:
Martin Pieuchot <mpi@openbsd.org>
Cc:
gameoftrees@openbsd.org
Date:
Sun, 19 Jan 2020 14:24:11 +0100

Download raw body.

Thread
  • Martin Pieuchot:

    double lock in tog(1)

    • Stefan Sperling:

      double lock in tog(1)

On Sun, Jan 19, 2020 at 02:16:49PM +0100, Martin Pieuchot wrote:
> If match_commit() fails there's no point in trying to re-grab the mutex
> that the thread already owns.  Simply stop the iteration after having
> release the lock.
> 
> Ok?
> 
> diff --git tog/tog.c tog/tog.c
> index db59543..fc01887 100644
> --- tog/tog.c
> +++ tog/tog.c
> @@ -1414,20 +1414,16 @@ queue_commits(struct got_commit_graph *graph, struct commit_queue *commits,
>  		commits->ncommits++;
>  
>  		if (*searching == TOG_SEARCH_FORWARD && !*search_next_done) {
>  			err = match_commit(&have_match, id, commit, regex);
> -			if (err) {
> -				pthread_mutex_lock(&tog_mutex);

This was supposed to be an unlock, so we could also just do s/_lock/_unlock/.
Anyway, your diff is ok.

> -				break;
> -			}
>  		}
>  
>  		errcode = pthread_mutex_unlock(&tog_mutex);
>  		if (errcode && err == NULL)
>  			err = got_error_set_errno(errcode,
>  			    "pthread_mutex_unlock");
>  
> -		if (have_match)
> +		if (err || have_match)
>  			break;
>  	}
>  
>  	return err;
> 
>