On 10/17/2011 08:00 PM, Junio C Hamano wrote:
> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>> On 10/12/2011 09:19 PM, Junio C Hamano wrote:
>>> Michael Haggerty <mhagger@xxxxxxxxxxxx> writes:
>>> ...
>>> Probably that is what all the existing callers want, but I would have
>>> expected that an existing feature would be kept, perhaps like this
>>> instead:
>>>
>>> if (!submodule) {
>>> struct ref_cache *c;
>>> for (c = ref_cache; c; c = c->next)
>>> clear_ref_cache(c);
>>> } else {
>>> clear_ref_cache(get_ref_cache(submodule);
>>> }
>> ...
>> Your specific suggestion would not work because currently
>> submodule==NULL signifies the main module. However, it would be easy to
>> add the few-line function when/if it is needed.
>
> I think "submodule==NULL" is probably a mistake; "" would make more sense
> given that you are storing the string in name[FLEX_ARRAY] field.
Sorry I didn't respond to this earlier.
The public API convention (which predates my changes) is that "char
*submodule" arguments either point at the relative path to the submodule
or are NULL to denote the main module. But since these are stored
internally in a name[FLEX_ARRAY] field, I have been using "" internally
to denote the main module. I believe that everything is done correctly,
but I admit that the use of different conventions internally and
externally is a potential source of programming errors.
If this is viewed as something that needs changing, the easiest thing
would probably be to add a "const char *submodule" in the ref_cache data
structure that either contains NULL or points at the name field, and to
consistently use the convention that the main module must always be
denoted by NULL. The space overhead would be negligible because the
number of ref_cache objects is limited to the number of submodules plus 1.
Michael
--
Michael Haggerty
mhagger@xxxxxxxxxxxx
http://softwareswirl.blogspot.com/
--
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