Re: [PATCH v1 01/27] revision: factor out parsing of diff-merge related options

> Sergey Organov <sorganov@gmail.com> writes:
>
>>>> +		if (!strcmp(optarg, "off")) {
>>>> +			revs->ignore_merges = 1;
>>>> +		} else {
>>>> +			die(_("unknown value for --diff-merges: %s"), optarg);
>>>> +		}
>>>
>>> To correct the above bug, it probably is sufficient to add
>>>
>>> 		return argcount;
>>>
>>> here.
>>
>> Right, but not enough. "argcount" should also be set to 1 at the
>> beginning of the function, to avoid returning uninitialized value here.
>
> You seem to be a bit confused.
>
> The suggested fix is to ...
>
> +static int parse_diff_merge_opts(struct rev_info *revs, const char **argv) {
> +	int argcount;
> +	const char *optarg;
> +	const char *arg = argv[0];
> +
> +	if (!strcmp(arg, "-m")) {
> +		/*
> +		 * To "diff-index", "-m" means "match missing", and to the "log"
> +		 * family of commands, it means "show full diff for merges". Set
> +		 * both fields appropriately.
> +		 */
> +		revs->ignore_merges = 0;
> +		revs->match_missing = 1;
> +	} else if (!strcmp(arg, "-c")) {
> +		revs->diff = 1;
> +		revs->dense_combined_merges = 0;
> +		revs->combine_merges = 1;
> +	} else if (!strcmp(arg, "--cc")) {
> +		revs->diff = 1;
> +		revs->dense_combined_merges = 1;
> +		revs->combine_merges = 1;
> +	} else if (!strcmp(arg, "--no-diff-merges")) {
> +		revs->ignore_merges = 1;
> +	} else if (!strcmp(arg, "--combined-all-paths")) {
> +		revs->diff = 1;
> +		revs->combined_all_paths = 1;
> +	} else if ((argcount = parse_long_opt("diff-merges", argv, &optarg))) {
> +		if (!strcmp(optarg, "off")) {
> +			revs->ignore_merges = 1;
> +		} else {
> +			die(_("unknown value for --diff-merges: %s"), optarg);
> +		}
>
> ... add
>
> 		return argcount;
>
> here.  We know argcount has a valid value that was returned from
> parse_long_opt() at this point.