Re: Custom merge driver with no rename detection

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1401 bytes --]

Hi Felipe,

On Sun, 14 Feb 2016, Felipe Gonçalves Assis wrote:

> Attached is a quick and dirty patch that emulates the effect by
> allowing greater than 100% rename thresholds to mean "no-renames".

It is really hard to comment on attached patches.

First comment: the commit message is awfully empty, and lacks a sign-off.

>         /* user says num divided by scale and we say internally that
>          * is MAX_SCORE * num / scale.
>          */
> -       return (int)((num >= scale) ? MAX_SCORE : (MAX_SCORE * num / scale));
> +       return (int)(MAX_SCORE * num / scale);

Uh oh. I suspect this opens the door pretty wide for integer overflows. I
could imagine that something like

	return (int)(num > scale ? MAX_SCORE + 1 : MAX_SCORE * num / scale);

would work better, but it still would need some careful consideration.

>  static int diff_scoreopt_parse(const char *opt)
> diff --git a/diffcore-rename.c b/diffcore-rename.c
> index af1fe08..7cb5a3b 100644
> --- a/diffcore-rename.c
> +++ b/diffcore-rename.c
> @@ -497,7 +497,7 @@ void diffcore_rename(struct diff_options *options)
>                         register_rename_src(p);
>                 }
>         }
> -       if (rename_dst_nr == 0 || rename_src_nr == 0)
> +       if (rename_dst_nr == 0 || rename_src_nr == 0 || minimum_score > MAX_SCORE)

This line is too long now.

Ciao,
Johannes