Johannes Schindelin <Johannes.Schindelin@xxxxxx> writes:
>> The program can decide at runtime not to use threading even if the support
>> is compiled in. In such a case, mutexes are not necessary and left
>> uninitialized. But the code incorrectly tried to take and release the
>> read_sha1_mutex unconditionally.
>>
>> Signed-off-by: Junio C Hamano <gitster@xxxxxxxxx>
>> Acked-by: Fredrik Kuivinen <frekui@xxxxxxxxx>
>> ---
>
> Yes, this one looks much, much nicer.
The structure may be much nicer, but one remaining thing is that I do not
think foo_locked() is a good name; IIRC, kernel folks use _locked() suffix
when the caller is expected to already hold the lock. So a typical naming
convention goes like this:
foo()
{
lock();
foo_locked();
unlock();
}
but what the patch did was the other way around:
read_sha1_file_locked()
{
lock();
read_sha1_file();
unlock();
}
which is probably against the convention many readers of our codebase are
already familiar with.
We need a better name to unconfuse people, I think.
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html