Re: [PATCH 1/5] check-ignore: move setup into cmd_check_ignore()

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]