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

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;
> 
>