Re: [RFC v1] Add virtual file system settings and hook proc

[Junio C Hamano <gitster@pobox.com>]

Ben Peart <peartben@gmail.com> writes:

>>> +	if (*dtype == DT_UNKNOWN)
>>> +		*dtype = get_dtype(NULL, istate, pathname, pathlen);
>>
>> We try to defer paying cost to determine unknown *dtype as late as
>> possible by having this call in last_exclude_matching_from_list(),
>> and not here.  If we are doing this, we probably should update the
>> callpaths that call last_exclude_matching_from_list() to make the
>> caller responsible for doing get_dtype() and drop the lazy finding
>> of dtype from the callee.  Alternatively, the new "is excluded from
>> vfs" helper can learn to do the lazy get_dtype() just like the
>> existing last_exclude_matching_from_list() does.  I suspect the
>> latter may be simpler.
>
> In is_excluded_from_virtualfilesystem() dtype can't be lazy because it
> is always needed (which is why I test and die if it isn't known).  

You make a call to that function even when virtual-file-system hook
is not in use, i.e. instead of the caller saying

	if (is_vfs_in_use()) {
		*dtype = get_dtype(...);
                if (is_excluded_from_vfs(...) > 0)
			return 1;
	}

your caller makes an unconditional call to is_excluded_from_vfs().
Isn't that the only reason why you break the laziness of determining
dtype?

You can keep the caller simple by making an unconditional call, but
maintain the laziness by updating the callig convention to pass
dtype (not *dtype) to the function, e.g..

	if (is_excluded_from_vfs(pathname, pathlen, dtype) > 0)
		return 1;

and then at the beginning of the helper

	if (is_vfs_in_use())
		return -1; /* undetermined */
	*dtype = get_dtype(...);
	... whatever logic it has now ...

no?

> Your comments are all feedback on the code - how it was implemented,
> style, etc.  Any thoughts on whether this is something we could/should
> merge into master (after any necessary cleanup)?  Would anyone else
> find this interesting/helpful?

I am pretty much neutral.  Not strongly opposed to it, but not all
that interested until seeing its integration with the "userland" to
see how the whole thing works ;-)