Subject: Re: [PATCH] fix threaded grep for machines with only one cpu

From: Junio C Hamano <[email-masked]>
Date: Tue, 16 Feb 2010 17:26:06 -0800

Johannes Schindelin <[email-masked]> 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 <[email-masked]>
>> Acked-by: Fredrik Kuivinen <[email-masked]>
>> ---
>
> 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.