git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] object_id.cocci: match only expressions of type 'struct object_id'

From
SZEDER Gábor <szeder.dev@gmail.com>
Date
Oct 12, 2018, 13:11 UTC
Message-ID
<20181012131116.23733-1-szeder.dev@gmail.com>
In-Reply-To
<20181008215701.779099-15-sandals@crustytoothpaste.net>

Most of our semantic patches in 'contrib/coccinelle/object_id.cocci' turn calls of SHA1-specific functions into calls of their corresponding object_id counterparts, e.g. sha1_to_hex() to oid_to_hex(). These semantic patches look something like this:

  @@
  expression E1;
  @@
  - sha1_to_hex(E1.hash)
  + oid_to_hex(&E1)

and match the access to the 'hash' field in any data type, not only in 'struct object_id', and, consquently, can produce wrong transformations.

Case in point is the recent hash function transition patch "rerere: convert to use the_hash_algo" [1], which, among other things, renamed 'struct rerere_dir's 'sha1' field to 'hash', and then 'make coccicheck' started to suggest the following wrong transformations for 'rerere.c' [2]:

  -    return sha1_to_hex(id->collection->hash);
  +    return oid_to_hex(id->collection);
and
  -    DIR *dir = opendir(git_path("rr-cache/%s", sha1_to_hex(rr_dir->hash)));
  +    DIR *dir = opendir(git_path("rr-cache/%s", oid_to_hex(rr_dir)));

Avoid such wrong transformations by tightening semantic patches in 'object_id.cocci' to match only type of or pointers to 'struct object_id'.

[1] https://public-inbox.org/git/20181008215701.779099-15-sandals@crustytoothpaste.net/ [2] https://travis-ci.org/git/git/jobs/440463476#L580

Signed-off-by: SZEDER Gábor <szeder.dev@gmail.com>
---

I think this patch should come before that patch mentioned in the commit message, or perhaps even before the whole patch series.

 contrib/coccinelle/object_id.cocci | 117 ++++++++++++++++-------------
 1 file changed, 63 insertions(+), 54 deletions(-)
diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index d8bdb48712..6a7cf3e02d 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -1,119 +1,127 @@
 @@
-expression E1;
+struct object_id OID;
 @@
-- is_null_sha1(E1.hash)
-+ is_null_oid(&E1)
+- is_null_sha1(OID.hash)
++ is_null_oid(&OID)
 
 @@
-expression E1;
+struct object_id *OIDPTR;
 @@
-- is_null_sha1(E1->hash)
-+ is_null_oid(E1)
+- is_null_sha1(OIDPTR->hash)
++ is_null_oid(OIDPTR)
 
 @@
-expression E1;
+struct object_id OID;
 @@
-- sha1_to_hex(E1.hash)
-+ oid_to_hex(&E1)
+- sha1_to_hex(OID.hash)
++ oid_to_hex(&OID)
 
 @@
 identifier f != oid_to_hex;
-expression E1;
+struct object_id *OIDPTR;
 @@
   f(...) {<...
-- sha1_to_hex(E1->hash)
-+ oid_to_hex(E1)
+- sha1_to_hex(OIDPTR->hash)
++ oid_to_hex(OIDPTR)
   ...>}
 
 @@
-expression E1, E2;
+expression E;
+struct object_id OID;
 @@
-- sha1_to_hex_r(E1, E2.hash)
-+ oid_to_hex_r(E1, &E2)
+- sha1_to_hex_r(E, OID.hash)
++ oid_to_hex_r(E, &OID)
 
 @@
 identifier f != oid_to_hex_r;
-expression E1, E2;
+expression E;
+struct object_id *OIDPTR;
 @@
    f(...) {<...
-- sha1_to_hex_r(E1, E2->hash)
-+ oid_to_hex_r(E1, E2)
+- sha1_to_hex_r(E, OIDPTR->hash)
++ oid_to_hex_r(E, OIDPTR)
   ...>}
 
 @@
-expression E1;
+struct object_id OID;
 @@
-- hashclr(E1.hash)
-+ oidclr(&E1)
+- hashclr(OID.hash)
++ oidclr(&OID)
 
 @@
 identifier f != oidclr;
-expression E1;
+struct object_id *OIDPTR;
 @@
   f(...) {<...
-- hashclr(E1->hash)
-+ oidclr(E1)
+- hashclr(OIDPTR->hash)
++ oidclr(OIDPTR)
   ...>}
 
 @@
-expression E1, E2;
+struct object_id OID1, OID2;
 @@
-- hashcmp(E1.hash, E2.hash)
-+ oidcmp(&E1, &E2)
+- hashcmp(OID1.hash, OID2.hash)
++ oidcmp(&OID1, &OID2)
 
 @@
 identifier f != oidcmp;
-expression E1, E2;
+struct object_id *OIDPTR1, OIDPTR2;
 @@
   f(...) {<...
-- hashcmp(E1->hash, E2->hash)
-+ oidcmp(E1, E2)
+- hashcmp(OIDPTR1->hash, OIDPTR2->hash)
++ oidcmp(OIDPTR1, OIDPTR2)
   ...>}
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
 @@
-- hashcmp(E1->hash, E2.hash)
-+ oidcmp(E1, &E2)
+- hashcmp(OIDPTR->hash, OID.hash)
++ oidcmp(OIDPTR, &OID)
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
 @@
-- hashcmp(E1.hash, E2->hash)
-+ oidcmp(&E1, E2)
+- hashcmp(OID.hash, OIDPTR->hash)
++ oidcmp(&OID, OIDPTR)
 
 @@
-expression E1, E2;
+struct object_id OID1, OID2;
 @@
-- hashcpy(E1.hash, E2.hash)
-+ oidcpy(&E1, &E2)
+- hashcpy(OID1.hash, OID2.hash)
++ oidcpy(&OID1, &OID2)
 
 @@
 identifier f != oidcpy;
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
 @@
   f(...) {<...
-- hashcpy(E1->hash, E2->hash)
-+ oidcpy(E1, E2)
+- hashcpy(OIDPTR1->hash, OIDPTR2->hash)
++ oidcpy(OIDPTR1, OIDPTR2)
   ...>}
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
 @@
-- hashcpy(E1->hash, E2.hash)
-+ oidcpy(E1, &E2)
+- hashcpy(OIDPTR->hash, OID.hash)
++ oidcpy(OIDPTR, &OID)
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR;
+struct object_id OID;
 @@
-- hashcpy(E1.hash, E2->hash)
-+ oidcpy(&E1, E2)
+- hashcpy(OID.hash, OIDPTR->hash)
++ oidcpy(&OID, OIDPTR)
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
 @@
-- oidcmp(E1, E2) == 0
-+ oideq(E1, E2)
+- oidcmp(OIDPTR1, OIDPTR2) == 0
++ oideq(OIDPTR1, OIDPTR2)
 
 @@
 identifier f != hasheq;
@@ -125,10 +133,11 @@ expression E1, E2;
   ...>}
 
 @@
-expression E1, E2;
+struct object_id *OIDPTR1;
+struct object_id *OIDPTR2;
 @@
-- oidcmp(E1, E2) != 0
-+ !oideq(E1, E2)
+- oidcmp(OIDPTR1, OIDPTR2) != 0
++ !oideq(OIDPTR1, OIDPTR2)
 
 @@
 identifier f != hasheq;
-- 
2.19.1.465.gaff195083f
Previous: Stefan BellerNext: Junio C Hamano
Message 28 of 30 in “Hash function transition part 15”
  1. 00/14 Hash function transition part 15brian m. carlson, Oct 8, 2018
  2. 01/14 pack-bitmap-write: use GIT_MAX_RAWSZ for allocationbrian m. carlson, Oct 8, 2018
  3. 02/14 builtin/repack: replace hard-coded constantbrian m. carlson, Oct 8, 2018
  4. Stefan BellerOct 8, 2018
  5. Eric SunshineOct 8, 2018
  6. brian m. carlsonOct 9, 2018
  7. 06/14 packfile: express constants in terms of the_hash_algobrian m. carlson, Oct 8, 2018
  8. Stefan BellerOct 8, 2018
  9. brian m. carlsonOct 9, 2018
  10. Stefan BellerOct 9, 2018
  11. brian m. carlsonOct 9, 2018
  12. 03/14 builtin/mktree: remove hard-coded constantbrian m. carlson, Oct 8, 2018
  13. Stefan BellerOct 8, 2018
  14. 04/14 builtin/fetch-pack: remove constants with parse_oid_hexbrian m. carlson, Oct 8, 2018
  15. 05/14 pack-revindex: express constants in terms of the_hash_algobrian m. carlson, Oct 8, 2018
  16. Stefan BellerOct 8, 2018
  17. brian m. carlsonOct 9, 2018
  18. 07/14 refs/packed-backend: express constants using the_hash_algobrian m. carlson, Oct 8, 2018
  19. 08/14 upload-pack: express constants in terms of the_hash_algobrian m. carlson, Oct 8, 2018
  20. 10/14 tag: express constant in terms of the_hash_algobrian m. carlson, Oct 8, 2018
  21. 09/14 transport: use parse_oid_hex instead of a constantbrian m. carlson, Oct 8, 2018
  22. 11/14 apply: replace hard-coded constantsbrian m. carlson, Oct 8, 2018
  23. 12/14 apply: rename new_sha1_prefix and old_sha1_prefixbrian m. carlson, Oct 8, 2018
  24. 13/14 submodule: make zero-oid comparison hash function agnosticbrian m. carlson, Oct 8, 2018
  25. Stefan BellerOct 8, 2018
  26. 14/14 rerere: convert to use the_hash_algobrian m. carlson, Oct 8, 2018
  27. Stefan BellerOct 8, 2018
  28. object_id.cocci: match only expressions of type 'struct object_id'SZEDER Gábor, Oct 12, 2018
  29. Junio C HamanoOct 15, 2018
  30. Junio C HamanoOct 15, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.