Junio C Hamano <gitster@xxxxxxxxx> writes:
> Sergey Organov <sorganov@xxxxxxxxx> 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.
[...]
OK, thanks, I now see what you meant, yet I now implemented it slightly
differently, as I finally need one point of return from the function
with non-zero value.
The end result is hopefully the same though.
Thanks,
-- Sergey Organov
