Re: [PATCH] blame.c: replace instance of !oidcmp for oideq

From: Jeff Smith <whydoubt@gmail.com>
Date: Sep 9, 2020, 17:48 UTC
Message-ID: <CAPX7N=5wRZbwWzzKmqyQGswJDFDf-DWq56pBPMSx4=fEuisskg@mail.gmail.com>
In-Reply-To: <CAOc6etZS3mGxsPPh25XFi2-qR0TNzq0Gx1NrydgQwmHbsjxejA@mail.gmail.com>

I haven't had a chance to look at the cocci script, but I did have one thought...

Derrick pointed out, 14438c4 added both oideq and hasheq. It might be good to have a similar check for hasheq, if there is not one already.

On Wed, Sep 9, 2020 at 9:01 AM Edmundo Carmona Antoranz <eantoranz@gmail.com> wrote:

Show 39 quoted lines

>
> On Wed, Sep 9, 2020 at 3:11 AM Jeff King <peff@peff.net> wrote:
> >
> > Yeah, it looks obviously correct. I am puzzled why "make coccicheck"
> > doesn't find this, though. +cc René, as my favorite target for
> > coccinelle nerd-snipes. :)
> >
>
> I added this to contrib/coccinelle/object_id.cocci in v2.27.0
>
> @@
> identifier f != oideq;
> expression E1, E2;
> @@
> - !oidcmp(E1, E2)
> + oideq(E1, E2)
>
> And it found it:
>
> $ cat contrib/coccinelle/object_id.cocci.patch
> diff -u -p a/blame.c b/blame.c
> --- a/blame.c
> +++ b/blame.c
> @@ -1352,8 +1352,7 @@ static struct blame_origin *find_origin(
>        else {
>                int compute_diff = 1;
>                if (origin->commit->parents &&
> -                   !oidcmp(&parent->object.oid,
> -                           &origin->commit->parents->item->object.oid))
> +                   oideq(&parent->object.oid,
> &origin->commit->parents->item->object.oid))
>                        compute_diff = maybe_changed_path(r, origin, bd);
>
>                if (compute_diff)
>
>
> Do I need to add more things into the coccinelle definition so that it
> is more restrictive in terms of the
> expression we are hunting down?