Re: [PATCH 29/30] create_dir_entry(): allow the flag value to be passed as an argument

[Michael Haggerty <mhagger@alum.mit.edu>]

On 04/25/2012 08:52 PM, Junio C Hamano wrote:
> Junio C Hamano<gitster@pobox.com>  writes:
>
>> mhagger@alum.mit.edu writes:
>>
>>> diff --git a/refs.c b/refs.c
>>> index 4eca965..869c9a7 100644
>>> --- a/refs.c
>>> +++ b/refs.c
>>> @@ -231,18 +231,18 @@ static void clear_ref_dir(struct ref_dir *dir)
>>>   }
>>>
>>>   /*
>>> + * Create a struct ref_entry object for the specified dirname and flag.
>>>    * dirname is the name of the directory with a trailing slash (e.g.,
>>>    * "refs/heads/") or "" for the top-level directory.
>>>    */
>>>   static struct ref_entry *create_dir_entry(struct ref_cache *ref_cache,
>>> -					  const char *dirname)
>>> +					  const char *dirname, int flag)
>>>   {
>>>   	struct ref_entry *direntry;
>>>   	int len = strlen(dirname);
>>>   	direntry = xcalloc(1, sizeof(struct ref_entry) + len + 1);
>>>   	memcpy(direntry->name, dirname, len + 1);
>>> -	direntry->flag = REF_DIR;
>>> +	direntry->flag = flag;
>>>   	direntry->u.subdir.ref_cache = ref_cache;
>>
>> As the returned structure will always represent a subdirectory and not a
>> leaf node, i.e. you use u.subdir, I do not think it makes any sense to
>> make it responsibility for the caller of this function to include
>> REF_DIR in the value of the flag.
>
> Forseeing a response "But but but REF_DIR will become OR of two
> variants", my complaint is still valid ;-) and it is the bit assignment
> you did in the final patch that is wrong.  If you make REF_DIR (or not)
> to differenticate between ref_dir vs ref_value, and use another bit
> REF_INCOMPLETE to remember that you still need to find out the actual
> value of it, the the above can still be

In an earlier design, there were three types of REF_DIR: packed, 
loose(read), and loose(unread), which could all be packed (along with 
dir/non-dir) into two bits.  The current bit assignment was left over 
from that design, plus a latent expectation that it would sometime be 
necessary to distinguish between packed and loose(read) references.

But so far I haven't found a reason to distinguish between packed and 
loose(read), so your suggested bit assignment is simpler.  I will 
include it in the next version of the patch series.

> The suggested bit assignment would also allow you to create ref_value
> leaf nodes, whose presence you know about (by iterating over readdir()
> results) but not their values yet (because you haven't opened and read
> them), by marking them with REF_INCOMPLETE to be extra lazy in the
> future, if necessary.  I do not know if that much laziness buys us a
> lot, though ;-).

This would be pretty easy to implement.  Maybe I'll try it someday.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/