[Cocci] Converting a function return type to void based on a SmPL constraint?

[Cocci] Converting a function return type to void based on a SmPL constraint?

[Markus Elfring]

Hello,

I have noticed that an update suggestion can be provided for
a source file by a known script for the semantic patch language.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/returnvar.cocci?id=8e0d0ad206f08506c893326ca7c9c3d9cc042cef

Update candidate:
btrfs_destroy_delayed_refs()
https://elixir.bootlin.com/linux/v5.4-rc2/source/fs/btrfs/disk-io.c#L4252
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/btrfs/disk-io.c?id=0e9d28bc6c81f47f94979ec7d0ab6ac42345fcee#n4243

I got the software development idea then to change the function return type
to “void” together with the deletion of return statements by the help of
the tool “Coccinelle 1.0.8-00004-g842075f7” instead of adjusting return values.

@replacement@
constant C;
identifier action, rc;
type T;
@@
 static
-T
+void
 action(...)
 {
-T rc = C;
 ... when != rc
     when strict
-return rc;
 }


Unfortunately, I do not get the transformation result which I would expect
for this approach. But I have noticed also that a similar SmPL script
can work as expected (if a function like “unpin_extent_cache” was not marked
as “static”?).
Would you like to add any advices?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Converting a function return type to void based on a SmPL constraint?

[Julia Lawall]

[-- Attachment #1: Type: text/plain, Size: 1929 bytes --]



On Tue, 15 Oct 2019, Markus Elfring wrote:

> Hello,
>
> I have noticed that an update suggestion can be provided for
> a source file by a known script for the semantic patch language.
> https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/returnvar.cocci?id=8e0d0ad206f08506c893326ca7c9c3d9cc042cef
>
> Update candidate:
> btrfs_destroy_delayed_refs()
> https://elixir.bootlin.com/linux/v5.4-rc2/source/fs/btrfs/disk-io.c#L4252
> https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/btrfs/disk-io.c?id=0e9d28bc6c81f47f94979ec7d0ab6ac42345fcee#n4243

Why are there two links here to the same thing???

>
> I got the software development idea then to change the function return type
> to “void” together with the deletion of return statements by the help of
> the tool “Coccinelle 1.0.8-00004-g842075f7” instead of adjusting return values.
>
> @replacement@
> constant C;
> identifier action, rc;
> type T;
> @@
>  static
> -T
> +void
>  action(...)
>  {
> -T rc = C;
>  ... when != rc
>      when strict
> -return rc;
>  }
>
>
> Unfortunately, I do not get the transformation result which I would expect
> for this approach. But I have noticed also that a similar SmPL script
> can work as expected (if a function like “unpin_extent_cache” was not marked
> as “static”?).
> Would you like to add any advices?

I don't know what result you expect, nor what is the relevance of
unpin_extent_cache, which is not the function linked to above.

My advice would be to take the time to cut down the example to a few lines
of code that can easily fit in an email, and then send a complete message
with no URLs or implicit references to previous emails that describes the
problem.

julia

> Regards,
> Markus
> _______________________________________________
> Cocci mailing list
> Cocci@systeme.lip6.fr
> https://systeme.lip6.fr/mailman/listinfo/cocci
>

[-- Attachment #2: Type: text/plain, Size: 136 bytes --]

_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Converting a function return type to void based on a SmPL constraint?

[Markus Elfring]

>> I got the software development idea then to change the function return type
>> to “void” together with the deletion of return statements by the help of
>> the tool “Coccinelle 1.0.8-00004-g842075f7” instead of adjusting return values.
>>
>> @replacement@
>> constant C;
>> identifier action, rc;
>> type T;
>> @@
>>  static
>> -T
>> +void
>>  action(...)
>>  {
>> -T rc = C;
>>  ... when != rc
>>      when strict
>> -return rc;
>>  }
>>
>>
>> Unfortunately, I do not get the transformation result which I would expect
>> for this approach. But I have noticed also that a similar SmPL script
>> can work as expected (if a function like “unpin_extent_cache” was not marked
>> as “static”?).
>> Would you like to add any advices?
>
> I don't know what result you expect,

I guess that you got similar expectations for the application of the file
“scripts/coccinelle/misc/returnvar.cocci”.


> nor what is the relevance of unpin_extent_cache,

It can be another example for the demonstration of our imaginations
around transformations at such source code places, can't it?


> which is not the function linked to above.

Additional links can be easily found.


> My advice would be to take the time to cut down the example to a few lines
> of code that can easily fit in an email,

What does hinder you to use a complete source code browser (or text editor)
based on one of the provided links?


> and then send a complete message with no URLs

Would you like to take another look at implementation details for
the mentioned update candidate?

Excerpt from the source file “fs/btrfs/disk-io.c” for your convenience:
…
static int btrfs_destroy_delayed_refs(struct btrfs_transaction *trans,
				      struct btrfs_fs_info *fs_info)
{
	struct rb_node *node;
	struct btrfs_delayed_ref_root *delayed_refs;
	struct btrfs_delayed_ref_node *ref;
	int ret = 0;

	delayed_refs = &trans->delayed_refs;

	spin_lock(&delayed_refs->lock);
	if (atomic_read(&delayed_refs->num_entries) == 0) {
		spin_unlock(&delayed_refs->lock);
		btrfs_info(fs_info, "delayed_refs has NO entry");
		return ret;
	}

…

	spin_unlock(&delayed_refs->lock);

	return ret;
}
…


> or implicit references to previous emails that describes the problem.

Where did you see additional email references for the initial clarification
request on this topic?


Will the application of the SmPL construct “<+... … ...+>” be needed for
such use cases?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

Re: [Cocci] Converting a function return type to void based on a SmPL constraint?

[Markus Elfring]

> Unfortunately, I do not get the transformation result which I would expect
> for this approach. But I have noticed also that a similar SmPL script
> can work as expected (if a function like “unpin_extent_cache” was not marked
> as “static”?).

The following SmPL script variant does also not work better at the moment.

@replacement@
constant C;
identifier action, rc;
type T;
@@
 static
-T
+void
 action(...)
 {
-T rc = C;
 <+... when != rc
       when strict
-return rc;
 ...+>
 }


Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci

[Cocci] Converting a function return type to void based on a SmPL constraint?

[Markus Elfring]

Hello,

I have noticed that an update suggestion can be provided for
a source file by a known script for the semantic patch language.
https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/scripts/coccinelle/misc/returnvar.cocci?id=8e0d0ad206f08506c893326ca7c9c3d9cc042cef

Update candidate:
btrfs_destroy_delayed_refs()
https://elixir.bootlin.com/linux/v5.4-rc2/source/fs/btrfs/disk-io.c#L4252
https://git.kernel.org/pub/scm/linux/kernel/git/next/linux-next.git/tree/fs/btrfs/disk-io.c?id=0e9d28bc6c81f47f94979ec7d0ab6ac42345fcee#n4243

I got the software development idea then to change the function return type
to “void” together with the deletion of return statements by the help of
the tool “Coccinelle 1.0.8-00004-g842075f7” instead of adjusting return values.

@replacement@
constant C;
identifier action, rc;
type T;
@@
 static
-T
+void
 action(...)
 {
-T rc = C;
 ... when != rc
     when strict
-return rc;
 }


Unfortunately, I do not get the transformation result which I would expect
for this approach. But I have noticed also that a similar SmPL script
can work as expected (if a function like “unpin_extent_cache” was not marked
as “static”?).
Would you like to add any advices?

Regards,
Markus
_______________________________________________
Cocci mailing list
Cocci@systeme.lip6.fr
https://systeme.lip6.fr/mailman/listinfo/cocci