threads / patch / 44302

patchRe: [PATCH] cocci: avoid self-references in object_id transformations

Subject: Re: [PATCH] cocci: avoid self-references in object_id transformations

## tl;dr

5 messages between Oct 16, 2016 and Oct 17, 2016. Diffs are folded; open one to read it.

replies: 4people: 3as markdown or json

René Scharfe· Oct 15, 2016, 08:25 UTC · lore

[PATCH] cocci: avoid self-references in object_id transformations

The object_id functions oid_to_hex, oid_to_hex_r, oidclr, oidcmp, and oidcpy are defined as wrappers of their legacy counterparts sha1_to_hex, sha1_to_hex_r, hashclr, hashcmp, and hashcpy, respectively. Make sure that the Coccinelle transformations for converting legacy function calls are not applied to these wrappers themselves, which would result in tautological declarations.

We can remove the added guards once the object_id functions stand on their own.

Signed-off-by: Rene Scharfe <l.s.r@web.de>
---
 contrib/coccinelle/object_id.cocci | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

Show changes to contrib/coccinelle/object_id.cocci +15 −0

diff --git a/contrib/coccinelle/object_id.cocci b/contrib/coccinelle/object_id.cocci
index 0307624..09afdbf 100644
--- a/contrib/coccinelle/object_id.cocci
+++ b/contrib/coccinelle/object_id.cocci
@@ -17,10 +17,13 @@ expression E1;
 + oid_to_hex(&E1)
 
 @@
+identifier f != oid_to_hex;
 expression E1;
 @@
+  f(...) {...
 - sha1_to_hex(E1->hash)
 + oid_to_hex(E1)
+  ...}
 
 @@
 expression E1, E2;
@@ -29,10 +32,13 @@ expression E1, E2;
 + oid_to_hex_r(E1, &E2)
 
 @@
+identifier f != oid_to_hex_r;
 expression E1, E2;
 @@
+   f(...) {...
 - sha1_to_hex_r(E1, E2->hash)
 + oid_to_hex_r(E1, E2)
+  ...}
 
 @@
 expression E1;
@@ -41,10 +47,13 @@ expression E1;
 + oidclr(&E1)
 
 @@
+identifier f != oidclr;
 expression E1;
 @@
+  f(...) {...
 - hashclr(E1->hash)
 + oidclr(E1)
+  ...}
 
 @@
 expression E1, E2;
@@ -53,10 +62,13 @@ expression E1, E2;
 + oidcmp(&E1, &E2)
 
 @@
+identifier f != oidcmp;
 expression E1, E2;
 @@
+  f(...) {...
 - hashcmp(E1->hash, E2->hash)
 + oidcmp(E1, E2)
+  ...}
 
 @@
 expression E1, E2;
@@ -77,10 +89,13 @@ expression E1, E2;
 + oidcpy(&E1, &E2)
 
 @@
+identifier f != oidcpy;
 expression E1, E2;
 @@
+  f(...) {...
 - hashcpy(E1->hash, E2->hash)
 + oidcpy(E1, E2)
+  ...}
 
 @@
 expression E1, E2;

-- 
2.10.1

brian m. carlson· Oct 15, 2016, 13:45 UTC · re: René Scharfe · lore

On Sat, Oct 15, 2016 at 10:25:34AM +0200, René Scharfe wrote:

Show 6 quoted lines

> The object_id functions oid_to_hex, oid_to_hex_r, oidclr, oidcmp, and
> oidcpy are defined as wrappers of their legacy counterparts sha1_to_hex,
> sha1_to_hex_r, hashclr, hashcmp, and hashcpy, respectively.  Make sure
> that the Coccinelle transformations for converting legacy function calls
> are not applied to these wrappers themselves, which would result in
> tautological declarations.

Ah, yes, this is a good idea. I've had to hack around this, but this is much better than having to fix it up by hand.

-- 
brian m. carlson / brian with sandals: Houston, Texas, US
+1 832 623 2791 | https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204

Junio C Hamano· Oct 17, 2016, 18:08 UTC · re: brian m. carlson · lore

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

Show 10 quoted lines

> On Sat, Oct 15, 2016 at 10:25:34AM +0200, René Scharfe wrote:
>> The object_id functions oid_to_hex, oid_to_hex_r, oidclr, oidcmp, and
>> oidcpy are defined as wrappers of their legacy counterparts sha1_to_hex,
>> sha1_to_hex_r, hashclr, hashcmp, and hashcpy, respectively.  Make sure
>> that the Coccinelle transformations for converting legacy function calls
>> are not applied to these wrappers themselves, which would result in
>> tautological declarations.
>
> Ah, yes, this is a good idea.  I've had to hack around this, but this is
> much better than having to fix it up by hand.

Yes, seeing an empty *.cocci.patch files after running coccicheck is a great feeling, but without something like this patch, we can never reach that goal ;-)

Thanks.

... oops. Totally unrelated to this patch, but I see these in strbuf.cocci.patch (this is at the tip of 'pu'), which are total nonsense. Perhaps I am running a way-stale spatch? It claims to be "spatch version 1.0.0-rc19 with Python support and with PCRE support"

--- date.c +++ /tmp/cocci-output-21568-bd3448-date.c

Show changes to diff +2 −2

@@ -179,7 +179,7 @@ const char *show_date(unsigned long time
 
 	if (mode->type == DATE_UNIX) {
 		strbuf_reset(&timebuf);
-		strbuf_addf(&timebuf, "%lu", time);
+		strbuf_addstr(&timebuf, time);
 		return timebuf.buf;
 	}
 
--- log-tree.c
+++ /tmp/cocci-output-21608-b02087-log-tree.c
@@ -400,7 +400,7 @@ void log_write_email_headers(struct rev_
 		extra_headers = subject_buffer;
 
 		if (opt->numbered_files)
-			strbuf_addf(&filename, "%d", opt->nr);
+			strbuf_addstr(&filename, opt->nr);
 		else
 			fmt_output_commit(&filename, commit, opt);
 		snprintf(buffer, sizeof(buffer) - 1,

René Scharfe· Oct 17, 2016, 20:04 UTC · re: Junio C Hamano · lore

Am 17.10.2016 um 20:08 schrieb Junio C Hamano:

Show 27 quoted lines

> ... oops.  Totally unrelated to this patch, but I see these in
> strbuf.cocci.patch (this is at the tip of 'pu'), which are total
> nonsense.  Perhaps I am running a way-stale spatch?  It claims to be
> "spatch version 1.0.0-rc19 with Python support and with PCRE support"
> 
> --- date.c
> +++ /tmp/cocci-output-21568-bd3448-date.c
> @@ -179,7 +179,7 @@ const char *show_date(unsigned long time
>  
>  	if (mode->type == DATE_UNIX) {
>  		strbuf_reset(&timebuf);
> -		strbuf_addf(&timebuf, "%lu", time);
> +		strbuf_addstr(&timebuf, time);
>  		return timebuf.buf;
>  	}
>  
> --- log-tree.c
> +++ /tmp/cocci-output-21608-b02087-log-tree.c
> @@ -400,7 +400,7 @@ void log_write_email_headers(struct rev_
>  		extra_headers = subject_buffer;
>  
>  		if (opt->numbered_files)
> -			strbuf_addf(&filename, "%d", opt->nr);
> +			strbuf_addstr(&filename, opt->nr);
>  		else
>  			fmt_output_commit(&filename, commit, opt);
>  		snprintf(buffer, sizeof(buffer) - 1,

I get these instead with 6513eabcbcbcfa684d4bb2d57f61c662b870b5ca on Debian testing with its "spatch version 1.0.4 with Python support and with PCRE support", which look legit:

--- sequencer.c +++ /tmp/cocci-output-40365-db7a71-sequencer.c

Show changes to diff +2 −2

@@ -159,7 +159,7 @@ int sequencer_remove_state(struct replay
 		free(opts->xopts[i]);
 	free(opts->xopts);
 
-	strbuf_addf(&dir, "%s", get_dir(opts));
+	strbuf_addstr(&dir, get_dir(opts));
 	remove_dir_recursively(&dir, 0);
 	strbuf_release(&dir);
 
--- builtin/branch.c
+++ /tmp/cocci-output-40858-a86d1a-branch.c
@@ -316,7 +316,7 @@ static char *build_format(struct ref_fil
 
 	if (filter->verbose) {
 		strbuf_addf(&local, "%%(align:%d,left)%%(refname:strip=2)%%(end)", maxwidth);
-		strbuf_addf(&local, "%s", branch_get_color(BRANCH_COLOR_RESET));
+		strbuf_addstr(&local, branch_get_color(BRANCH_COLOR_RESET));
 		strbuf_addf(&local, " %%(objectname:short=7) ");
 
 		if (filter->verbose > 1)

Junio C Hamano· Oct 17, 2016, 21:11 UTC · re: René Scharfe · lore

René Scharfe <l.s.r@web.de> writes:

> I get these instead with 6513eabcbcbcfa684d4bb2d57f61c662b870b5ca on
> Debian testing with its "spatch version 1.0.4 with Python support and
> with PCRE support", which look legit:

They do look good. So I'd stop worrying about it and see how painful to update my copy of spatch would be.

Thanks.