Re: [PATCH v2 2/2] hex: make hash_to_hex_algop() and friends thread-safe

On Thu, Jul 16, 2020 at 9:56 AM Johannes Schindelin [off-list ref] wrote:

Now, I am _far_ from knowing what I'm doing with Coccinelle, but I think
this here semantic patch should get you going:

-- snipsnap --
@@
expression E;
@@
  {
++   char hex[GIT_MAX_HEXSZ + 1];
     ...
-    oid_to_hex(E)
+    oid_to_hex_r(hex, E)
     ...
  }

@@
expression E1, E2;
@@
  {
++   char hex1[GIT_MAX_HEXSZ + 1], hex2[GIT_MAX_HEXSZ + 1];
     ...
-    oid_to_hex(E1)
+    oid_to_hex_r(hex1, E1)
     ...
-    oid_to_hex(E2)
+    oid_to_hex_r(hex2, E2)
     ...
  }

Thanks for this nice example! This already worked very well in some of
my tests :)

However, with my _very_ limited notion of Coccinelle, I didn't
understand why some code snippets didn't match the above rules. For
example, the structure below:

func(...)
{
	if (cond)
		func2("%s", oid_to_hex(a));
}

I thought it could be because the `if` statement is missing the curly
brackets (and it does work if I add the brackets), but to my surprise,
adding another oid_to_hex() call in an `else` case also made the code
match the rule:

func(...)
{
	if (cond)
		func2("%s", oid_to_hex(a));
	else
		func2("%s", oid_to_hex(a));
}

The following snippet also correctly matches, but spatch introduces only
one `hex` variable:

	if (cond)
		func2("%s, %s", oid_to_hex(a), oid_to_hex(b));
	else
		func2("%s", oid_to_hex(a));

We will probably want our semantic rules to handle an arbitrary number
of `oid_to_hex()` calls in each function, but in scenarios like the
above one, we only really need 2 hex buffers despite having 3 calls...
That might be a little tricky, I guess.

Another thing that might be tricky in this conversion is checking for
name conflicts with the added `hex` variable (but maybe Coccinelle
already has a facilitator mechanism for such cases? IDK).