On Thu, Apr 11, 2013 at 02:59:31AM +0100, Adam Spiers wrote: > Initialisation of the dir_struct and path_exclude_check structs was > previously done within check_ignore(). This was acceptable since > check_ignore() was only called once per check-ignore invocation; > however the next commit will convert it into an inner loop which is > called once per line of STDIN when --stdin is given. Therefore moving > the initialisation code out into cmd_check_ignore() ensures that > initialisation is still only performed once per check-ignore > invocation, and consequently that the output is identical whether > pathspecs are provided as CLI arguments or via STDIN. > > Signed-off-by: Adam Spiers <g...@adamspiers.org> > --- > builtin/check-ignore.c | 39 ++++++++++++++++++++------------------- > 1 file changed, 20 insertions(+), 19 deletions(-) > > diff --git a/builtin/check-ignore.c b/builtin/check-ignore.c > index 0240f99..0a4eef1 100644 > --- a/builtin/check-ignore.c > +++ b/builtin/check-ignore.c > @@ -53,30 +53,20 @@ static void output_exclude(const char *path, struct > exclude *exclude) > } > } > > -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. -Peff -- 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
