On Thu, Apr 11, 2013 at 01:25:53AM -0400, Jeff King wrote: > On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote: > > -static int check_ignore(const char *prefix, const char **pathspec) > > +static int check_ignore(struct path_exclude_check check, > > + const char *prefix, const char **pathspec) > > Did you mean to pass the struct by value here? If it is truly a per-path > value, shouldn't it be declared and initialized inside here? Otherwise > you risk one invocation munging things that the struct points to, but > the caller's copy does not know about the change. > > In particular, I see that the struct includes a strbuf. What happens > when one invocation of check_ignore grows the strbuf, then returns? The > copy of the struct in the caller will not know that the buffer it is > pointing to is now bogus. > > > -static int check_ignore_stdin_paths(const char *prefix) > > +static int check_ignore_stdin_paths(struct path_exclude_check check, const > > char *prefix) > > Ditto here.
It's not a per-path value; it's supposed to be reused across checks for multiple paths, as explained in the comments above last_exclude_matching_path(): ... * A path to a directory known to be excluded is left in check->path to * optimize for repeated checks for files in the same excluded directory. */ struct exclude *last_exclude_matching_path(struct path_exclude_check *check, ... So I think you're probably right that there is potential for check->path to become effectively corrupted due to the caller not seeing the reallocation. I'll change this too. -- To unsubscribe from this list: send the line "unsubscribe git" in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html