On Thu, Feb 02, 2012 at 10:34:07AM -0800, Junio C Hamano wrote:
> Jeff King <peff@xxxxxxxx> writes:
>
> > - grep_attr_lock();
> > - drv = userdiff_find_by_path(gs->name);
> > - grep_attr_unlock();
> > - if (drv && drv->funcname.pattern) {
> > - const struct userdiff_funcname *pe = &drv->funcname;
> > + grep_source_load_driver(gs);
> > + if (gs->driver->funcname.pattern) {
> > + const struct userdiff_funcname *pe = &gs->driver->funcname;
>
> When we load driver, gs->driver gets at least "default" driver, so we no
> longer need to check for drv != NULL as we used to? Is that the reason
> for the slight difference here?
Yes, exactly.
We could just leave gs->driver NULL instead of looking up "default", and
then use NULL to signal to the calling code that defaults should be
used. But NULL is interpreted by grep_source_load_driver as "we did not
look up the driver yet", so the common case of "no driver" would mean we
accidentally do the lookup multiple times. The diff_filespec code uses
the same convention to solve the same problem.
Speaking of which, there was some notion in my mind that a "grep_source"
and a "diff_filespec" were very similar objects, and that we could
possibly unify the implementations. I decided against that route with
this series, as it would have involved pretty heavy refactoring of the
diff code to prevent a fairly small amount of code duplication.
-Peff
--
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