Re: Custom merge driver with no rename detection

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

 



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

[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]