Re: [PATCH] merge-recursive: fix parsing of "diff-algorithm" option

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

 



John Keeping wrote:

> The "diff-algorithm" option to the recursive merge strategy takes the
> name of the algorithm as an option, but it uses strcmp on the option
> string to check if it starts with "diff-algorithm=", meaning that this
> options cannot actually be used.
>
> Fix this by switching to prefixcmp.  At the same time, clarify the
> following line by using strlen instead of a hard-coded length, which
> also makes it consistent with nearby code.
>
> Reported-by: Luke Noel-Storr <luke.noel-storr@xxxxxxxxxxxxxxx>
> Signed-off-by: John Keeping <john@xxxxxxxxxxxxx>

Thanks, both.

[...]
> --- a/merge-recursive.c
> +++ b/merge-recursive.c
> @@ -2069,8 +2069,8 @@ int parse_merge_opt(struct merge_options *o, const char *s)
>  		o->xdl_opts = DIFF_WITH_ALG(o, PATIENCE_DIFF);
>  	else if (!strcmp(s, "histogram"))
>  		o->xdl_opts = DIFF_WITH_ALG(o, HISTOGRAM_DIFF);
> -	else if (!strcmp(s, "diff-algorithm=")) {
> -		long value = parse_algorithm_value(s+15);
> +	else if (!prefixcmp(s, "diff-algorithm=")) {
> +		long value = parse_algorithm_value(s + strlen("diff-algorithm="));
>  		if (value < 0)
>  			return -1;

While we're here:

Part of the problem is that there are no tests for this option (or for
'git diff --diff-algorithm', or for the '[diff] algorithm'
configuration), so we didn't notice it didn't work until someone
actually tried it.

Do you have any examples of a diff or merge that works better with
some particular diff algorithm, that could be used in tests?

Ciao,
Jonathan
--
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]