From: Johannes Sixt <j6t@kdbg.org>
Date: Wed, 12 Oct 2016 20:07:03 GMT
Subject: Re: [PATCHv2] attr: convert to new threadsafe API
Message-ID: <44c554b8-7ac1-047d-59f0-b4d5331ed496@kdbg.org>
In-Reply-To: <20161011235951.8358-1-sbeller@google.com>

Am 12.10.2016 um 01:59 schrieb Stefan Beller:
> +void git_attr_check_initl(struct git_attr_check **check_,
> +			  const char *one, ...)
>  {
> -	struct git_attr_check *check;
>  	int cnt;
>  	va_list params;
>  	const char *param;
> +	struct git_attr_check *check;
> +
> +	if (*check_)
> +		return;
> +
> +	attr_lock();
> +	if (*check_) {
> +		attr_unlock();
> +		return;
> +	}
> ...
>  	check = xcalloc(1,
> -			sizeof(*check) + cnt * sizeof(*(check->check)));
> +			sizeof(*check) + cnt * sizeof(*(check->attr)));
> ...
> +	*check_ = check;
> +	attr_unlock();

Sigh. DCLP, the Double Checked Locking Pattern. These days, it should be 
common knowledge among professionals that this naïve version 
_does_not_work_ [1]!

I suggest you go without it, then measure, and only *then* optimize if 
it is a bottleneck. Did I read "we do not expect much contention" somewhere?

[1] http://www.aristeia.com/Papers/DDJ_Jul_Aug_2004_revised.pdf C++ 
centric, but applies to C just as well

-- Hannes