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 majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html