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

[Jeff King <peff@xxxxxxxx>]

On Wed, Sep 09, 2020 at 03:13:46PM -0400, Jeff King wrote:

> Which really _seems_ like a bug in coccinelle, unless I am missing
> something. Because both of those parameters look like object_id pointers
> (and the compiler would be complaining if it were not the case).  But I
> also wonder if giving the specific types in the coccinelle rule is
> buying us anything. If you passed two void pointers or ints or whatever
> to !oidcmp(), we'd still want to rewrite it as oideq().

And indeed, just blindly swapping out "struct object_id" for
"expression" in the coccinelle file (patch below), shows another spot
that was missed:

diff -u -p a/packfile.c b/packfile.c
--- a/packfile.c
+++ b/packfile.c
@@ -735,7 +735,7 @@ struct packed_git *add_packed_git(const
 	p->mtime = st.st_mtime;
 	if (path_len < the_hash_algo->hexsz ||
 	    get_sha1_hex(path + path_len - the_hash_algo->hexsz, p->hash))
-		hashclr(p->hash);
+		oidclr(p);
 	return p;
 }
 

Maybe it's worth being looser in our cocci patch definitions. I'm having
trouble thinking of a downside...

-Peff

-- >8 --
Here's the patch to loosen object_id.cocci. Perhaps we'd want to do the
same in other files.

diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index ddf4f22bd7..738c60923e 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -1,62 +1,62 @@
 @@
-struct object_id OID;
+expression OID;
 @@
 - is_null_sha1(OID.hash)
 + is_null_oid(&OID)
 
 @@
-struct object_id *OIDPTR;
+expression *OIDPTR;
 @@
 - is_null_sha1(OIDPTR->hash)
 + is_null_oid(OIDPTR)
 
 @@
-struct object_id OID;
+expression OID;
 @@
 - hashclr(OID.hash)
 + oidclr(&OID)
 
 @@
 identifier f != oidclr;
-struct object_id *OIDPTR;
+expression *OIDPTR;
 @@
   f(...) {<...
 - hashclr(OIDPTR->hash)
 + oidclr(OIDPTR)
   ...>}
 
 @@
-struct object_id OID1, OID2;
+expression OID1, OID2;
 @@
 - hashcmp(OID1.hash, OID2.hash)
 + oidcmp(&OID1, &OID2)
 
 @@
 identifier f != oidcmp;
-struct object_id *OIDPTR1, OIDPTR2;
+expression *OIDPTR1, OIDPTR2;
 @@
   f(...) {<...
 - hashcmp(OIDPTR1->hash, OIDPTR2->hash)
 + oidcmp(OIDPTR1, OIDPTR2)
   ...>}
 
 @@
-struct object_id *OIDPTR;
-struct object_id OID;
+expression *OIDPTR;
+expression OID;
 @@
 - hashcmp(OIDPTR->hash, OID.hash)
 + oidcmp(OIDPTR, &OID)
 
 @@
-struct object_id *OIDPTR;
-struct object_id OID;
+expression *OIDPTR;
+expression OID;
 @@
 - hashcmp(OID.hash, OIDPTR->hash)
 + oidcmp(&OID, OIDPTR)
 
 @@
-struct object_id *OIDPTR1;
-struct object_id *OIDPTR2;
+expression OIDPTR1;
+expression OIDPTR2;
 @@
 - oidcmp(OIDPTR1, OIDPTR2) == 0
 + oideq(OIDPTR1, OIDPTR2)
@@ -71,8 +71,8 @@ expression E1, E2;
   ...>}
 
 @@
-struct object_id *OIDPTR1;
-struct object_id *OIDPTR2;
+expression *OIDPTR1;
+expression *OIDPTR2;
 @@
 - oidcmp(OIDPTR1, OIDPTR2) != 0
 + !oideq(OIDPTR1, OIDPTR2)