2022-03-24 15:42:57

by Jakob Koschel

Subject: [PATCH] char: xillybus: replace usage of found with dedicated list iterator variable

To move the list iterator variable into the list_for_each_entry_*()
macro in the future it should be avoided to use the list iterator
variable after the loop body.

To *never* use the list iterator variable after the loop it was
concluded to use a separate iterator variable instead of a
found boolean [1].

This removes the need to use a found variable and simply checking if
the variable was set, can determine if the break/goto was hit.

Link: https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
Signed-off-by: Jakob Koschel <[email protected]>
---
drivers/char/xillybus/xillybus_class.c | 26 ++++++++++++--------------
1 file changed, 12 insertions(+), 14 deletions(-)

diff --git a/drivers/char/xillybus/xillybus_class.c b/drivers/char/xillybus/xillybus_class.c
index 5046486011c8..0f238648dcfe 100644
--- a/drivers/char/xillybus/xillybus_class.c
+++ b/drivers/char/xillybus/xillybus_class.c
@@ -174,18 +174,17 @@ void xillybus_cleanup_chrdev(void *private_data,
struct device *dev)
{
int minor;
- struct xilly_unit *unit;
- bool found = false;
+ struct xilly_unit *unit = NULL, *iter;

mutex_lock(&unit_mutex);

- list_for_each_entry(unit, &unit_list, list_entry)
- if (unit->private_data == private_data) {
- found = true;
+ list_for_each_entry(iter, &unit_list, list_entry)
+ if (iter->private_data == private_data) {
+ unit = iter;
break;
}

- if (!found) {
+ if (!unit) {
dev_err(dev, "Weird bug: Failed to find unit\n");
mutex_unlock(&unit_mutex);
return;
@@ -216,22 +215,21 @@ int xillybus_find_inode(struct inode *inode,
{
int minor = iminor(inode);
int major = imajor(inode);
- struct xilly_unit *unit;
- bool found = false;
+ struct xilly_unit *unit = NULL, *iter;

mutex_lock(&unit_mutex);

- list_for_each_entry(unit, &unit_list, list_entry)
- if (unit->major == major &&
- minor >= unit->lowest_minor &&
- minor < (unit->lowest_minor + unit->num_nodes)) {
- found = true;
+ list_for_each_entry(iter, &unit_list, list_entry)
+ if (iter->major == major &&
+ minor >= iter->lowest_minor &&
+ minor < (iter->lowest_minor + iter->num_nodes)) {
+ unit = iter;
break;
}

mutex_unlock(&unit_mutex);

- if (!found)
+ if (!unit)
return -ENODEV;

*private_data = unit->private_data;

base-commit: f443e374ae131c168a065ea1748feac6b2e76613
--
2.25.1

2022-03-25 18:18:26

by Eli Billauer

Subject: Re: [PATCH] char: xillybus: replace usage of found with dedicated list iterator variable

Thanks.

Acked-by: Eli Billauer <[email protected]>

On 24/03/22 09:09, Jakob Koschel wrote:
> To move the list iterator variable into the list_for_each_entry_*()
> macro in the future it should be avoided to use the list iterator
> variable after the loop body.
>
> To *never* use the list iterator variable after the loop it was
> concluded to use a separate iterator variable instead of a
> found boolean [1].
>
> This removes the need to use a found variable and simply checking if
> the variable was set, can determine if the break/goto was hit.
>
> Link:https://lore.kernel.org/all/CAHk-=wgRr_D8CB-D9Kg-c=EHreAsk5SqXPwr9Y7k9sA6cWXJ6w@mail.gmail.com/
> Signed-off-by: Jakob Koschel<[email protected]>
> ---
> drivers/char/xillybus/xillybus_class.c | 26 ++++++++++++--------------
> 1 file changed, 12 insertions(+), 14 deletions(-)
>
> diff --git a/drivers/char/xillybus/xillybus_class.c b/drivers/char/xillybus/xillybus_class.c
> index 5046486011c8..0f238648dcfe 100644
> --- a/drivers/char/xillybus/xillybus_class.c
> +++ b/drivers/char/xillybus/xillybus_class.c
> @@ -174,18 +174,17 @@ void xillybus_cleanup_chrdev(void *private_data,
> struct device *dev)
> {
> int minor;
> - struct xilly_unit *unit;
> - bool found = false;
> + struct xilly_unit *unit = NULL, *iter;
>
> mutex_lock(&unit_mutex);
>
> - list_for_each_entry(unit,&unit_list, list_entry)
> - if (unit->private_data == private_data) {
> - found = true;
> + list_for_each_entry(iter,&unit_list, list_entry)
> + if (iter->private_data == private_data) {
> + unit = iter;
> break;
> }
>
> - if (!found) {
> + if (!unit) {
> dev_err(dev, "Weird bug: Failed to find unit\n");
> mutex_unlock(&unit_mutex);
> return;
> @@ -216,22 +215,21 @@ int xillybus_find_inode(struct inode *inode,
> {
> int minor = iminor(inode);
> int major = imajor(inode);
> - struct xilly_unit *unit;
> - bool found = false;
> + struct xilly_unit *unit = NULL, *iter;
>
> mutex_lock(&unit_mutex);
>
> - list_for_each_entry(unit,&unit_list, list_entry)
> - if (unit->major == major&&
> - minor>= unit->lowest_minor&&
> - minor< (unit->lowest_minor + unit->num_nodes)) {
> - found = true;
> + list_for_each_entry(iter,&unit_list, list_entry)
> + if (iter->major == major&&
> + minor>= iter->lowest_minor&&
> + minor< (iter->lowest_minor + iter->num_nodes)) {
> + unit = iter;
> break;
> }
>
> mutex_unlock(&unit_mutex);
>
> - if (!found)
> + if (!unit)
> return -ENODEV;
>
> *private_data = unit->private_data;
>
> base-commit: f443e374ae131c168a065ea1748feac6b2e76613
>