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/ [1]
Signed-off-by: Jakob Koschel <[email protected]>
---
Changes in v2:
- fix checkpatch intentation issues
Note: I've changed my email address to contributions to
[email protected], I hope that's not an issue since the v1 was still
on my old email.
---
drivers/w1/w1.c | 32 +++++++++++++++-----------------
1 file changed, 15 insertions(+), 17 deletions(-)
diff --git a/drivers/w1/w1.c b/drivers/w1/w1.c
index 9d199fed9628..c453160684e1 100644
--- a/drivers/w1/w1.c
+++ b/drivers/w1/w1.c
@@ -830,49 +830,47 @@ int w1_slave_detach(struct w1_slave *sl)
struct w1_master *w1_search_master_id(u32 id)
{
- struct w1_master *dev;
- int found = 0;
+ struct w1_master *dev = NULL, *iter;
mutex_lock(&w1_mlock);
- list_for_each_entry(dev, &w1_masters, w1_master_entry) {
- if (dev->id == id) {
- found = 1;
- atomic_inc(&dev->refcnt);
+ list_for_each_entry(iter, &w1_masters, w1_master_entry) {
+ if (iter->id == id) {
+ dev = iter;
+ atomic_inc(&iter->refcnt);
break;
}
}
mutex_unlock(&w1_mlock);
- return (found)?dev:NULL;
+ return dev;
}
struct w1_slave *w1_search_slave(struct w1_reg_num *id)
{
struct w1_master *dev;
- struct w1_slave *sl = NULL;
- int found = 0;
+ struct w1_slave *sl = NULL, *iter;
mutex_lock(&w1_mlock);
list_for_each_entry(dev, &w1_masters, w1_master_entry) {
mutex_lock(&dev->list_mutex);
- list_for_each_entry(sl, &dev->slist, w1_slave_entry) {
- if (sl->reg_num.family == id->family &&
- sl->reg_num.id == id->id &&
- sl->reg_num.crc == id->crc) {
- found = 1;
+ list_for_each_entry(iter, &dev->slist, w1_slave_entry) {
+ if (iter->reg_num.family == id->family &&
+ iter->reg_num.id == id->id &&
+ iter->reg_num.crc == id->crc) {
+ sl = iter;
atomic_inc(&dev->refcnt);
- atomic_inc(&sl->refcnt);
+ atomic_inc(&iter->refcnt);
break;
}
}
mutex_unlock(&dev->list_mutex);
- if (found)
+ if (sl)
break;
}
mutex_unlock(&w1_mlock);
- return (found)?sl:NULL;
+ return sl;
}
void w1_reconnect_slaves(struct w1_family *f, int attach)
---
base-commit: eeac8ede17557680855031c6f305ece2378af326
change-id: 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-c56393ff0339
Best regards,
--
Jakob Koschel <[email protected]>
On 10/05/2023 00:49, 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/ [1]
> Signed-off-by: Jakob Koschel <[email protected]>
> ---
> Changes in v2:
> - fix checkpatch intentation issues
I don't see the differences and checkpatch still complains. Are you sure
you sent v2?
Best regards,
Krzysztof
How strange, I just checked and my checkpatch doesn't complain.
I also redownloaded and double checked
(b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com).
What's the exact issue you are seeing?
- jakob
On 23/05/12 09:27AM, Krzysztof Kozlowski wrote:
> On 10/05/2023 00:49, 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/ [1]
> > Signed-off-by: Jakob Koschel <[email protected]>
> > ---
> > Changes in v2:
> > - fix checkpatch intentation issues
>
> I don't see the differences and checkpatch still complains. Are you sure
> you sent v2?
>
> Best regards,
> Krzysztof
>
On 12/05/2023 12:19, Jakob Koschel wrote:
> How strange, I just checked and my checkpatch doesn't complain.
>
> I also redownloaded and double checked
> (b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com).
>
> What's the exact issue you are seeing?
✓ [PATCH v2] w1: Replace usage of found with dedicated list iterator
variable
+ Link:
https://lore.kernel.org/r/20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
+ Signed-off-by: Krzysztof Kozlowski <[email protected]>
---
✗ No key: ed25519/[email protected]
✓ Signed: DKIM/gmail.com
---
Total patches: 1
---
Base: using specified base-commit eeac8ede17557680855031c6f305ece2378af326
Applying: w1: Replace usage of found with dedicated list iterator variable
[Checking commit] 12b61e664c29 w1: Replace usage of found with
dedicated list iterator variable
[Checkpatch]
CHECK: Alignment should match open parenthesis
#70: FILE: drivers/w1/w1.c:849:
+ if (iter->reg_num.family == id->family &&
+ iter->reg_num.id == id->id &&
Best regards,
Krzysztof
On 12/05/2023 13:17, Jakob Koschel wrote:
> On 23/05/12 12:26PM, Krzysztof Kozlowski wrote:
>> On 12/05/2023 12:19, Jakob Koschel wrote:
>>> How strange, I just checked and my checkpatch doesn't complain.
>>>
>>> I also redownloaded and double checked
>>> (b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com).
>>>
>>> What's the exact issue you are seeing?
>>
>> ✓ [PATCH v2] w1: Replace usage of found with dedicated list iterator
>> variable
>> + Link:
>> https://lore.kernel.org/r/20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
>> + Signed-off-by: Krzysztof Kozlowski <[email protected]>
>> ---
>> ✗ No key: ed25519/[email protected]
>> ✓ Signed: DKIM/gmail.com
>> ---
>> Total patches: 1
>> ---
>> Base: using specified base-commit eeac8ede17557680855031c6f305ece2378af326
>> Applying: w1: Replace usage of found with dedicated list iterator variable
>> [Checking commit] 12b61e664c29 w1: Replace usage of found with
>> dedicated list iterator variable
>> [Checkpatch]
>> CHECK: Alignment should match open parenthesis
>> #70: FILE: drivers/w1/w1.c:849:
>> + if (iter->reg_num.family == id->family &&
>> + iter->reg_num.id == id->id &&
>
> I tried a bunch of things but I can't see this message. I tried with 'checkpatch.pl' from different kernel commits.
>
> I did:
>
> b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
> scripts/checkpatch.pl ./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx
>
> and always get:
>
> total: 0 errors, 0 warnings, 64 lines checked
>
> ./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx has no obvious style problems and is ready for submission.
>
First - the missing indentation is visible with bare eyes, you can also
fix it without checking in checkpatch.
It seems you are not running checkpatch with --strict. If you want to
see the exact error - run with strict.
Best regards,
Krzysztof
On 23/05/12 12:26PM, Krzysztof Kozlowski wrote:
> On 12/05/2023 12:19, Jakob Koschel wrote:
> > How strange, I just checked and my checkpatch doesn't complain.
> >
> > I also redownloaded and double checked
> > (b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com).
> >
> > What's the exact issue you are seeing?
>
> ✓ [PATCH v2] w1: Replace usage of found with dedicated list iterator
> variable
> + Link:
> https://lore.kernel.org/r/20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
> + Signed-off-by: Krzysztof Kozlowski <[email protected]>
> ---
> ✗ No key: ed25519/[email protected]
> ✓ Signed: DKIM/gmail.com
> ---
> Total patches: 1
> ---
> Base: using specified base-commit eeac8ede17557680855031c6f305ece2378af326
> Applying: w1: Replace usage of found with dedicated list iterator variable
> [Checking commit] 12b61e664c29 w1: Replace usage of found with
> dedicated list iterator variable
> [Checkpatch]
> CHECK: Alignment should match open parenthesis
> #70: FILE: drivers/w1/w1.c:849:
> + if (iter->reg_num.family == id->family &&
> + iter->reg_num.id == id->id &&
I tried a bunch of things but I can't see this message. I tried with 'checkpatch.pl' from different kernel commits.
I did:
b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
scripts/checkpatch.pl ./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx
and always get:
total: 0 errors, 0 warnings, 64 lines checked
./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx has no obvious style problems and is ready for submission.
I'm out of ideas...
>
>
>
> Best regards,
> Krzysztof
>
On 23/05/12 01:23PM, Krzysztof Kozlowski wrote:
> On 12/05/2023 13:17, Jakob Koschel wrote:
> > On 23/05/12 12:26PM, Krzysztof Kozlowski wrote:
> >> On 12/05/2023 12:19, Jakob Koschel wrote:
> >>> How strange, I just checked and my checkpatch doesn't complain.
> >>>
> >>> I also redownloaded and double checked
> >>> (b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com).
> >>>
> >>> What's the exact issue you are seeing?
> >>
> >> ✓ [PATCH v2] w1: Replace usage of found with dedicated list iterator
> >> variable
> >> + Link:
> >> https://lore.kernel.org/r/20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
> >> + Signed-off-by: Krzysztof Kozlowski <[email protected]>
> >> ---
> >> ✗ No key: ed25519/[email protected]
> >> ✓ Signed: DKIM/gmail.com
> >> ---
> >> Total patches: 1
> >> ---
> >> Base: using specified base-commit eeac8ede17557680855031c6f305ece2378af326
> >> Applying: w1: Replace usage of found with dedicated list iterator variable
> >> [Checking commit] 12b61e664c29 w1: Replace usage of found with
> >> dedicated list iterator variable
> >> [Checkpatch]
> >> CHECK: Alignment should match open parenthesis
> >> #70: FILE: drivers/w1/w1.c:849:
> >> + if (iter->reg_num.family == id->family &&
> >> + iter->reg_num.id == id->id &&
> >
> > I tried a bunch of things but I can't see this message. I tried with 'checkpatch.pl' from different kernel commits.
> >
> > I did:
> >
> > b4 am 20230509-w1-replace-usage-of-found-with-tmp-list-iterator-variable-v2-1-259bf1ba44ed@gmail.com
> > scripts/checkpatch.pl ./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx
> >
> > and always get:
> >
> > total: 0 errors, 0 warnings, 64 lines checked
> >
> > ./v2_20230510_jkl820_git_w1_replace_usage_of_found_with_dedicated_list_iterator_variable.mbx has no obvious style problems and is ready for submission.
> >
>
> First - the missing indentation is visible with bare eyes, you can also
> fix it without checking in checkpatch.
Ah the original code was already wrong and I just kept that way.
Since I'm not frequently contributing and didn't have an error to work with I also didn't know what the correct
way should be.
>
> It seems you are not running checkpatch with --strict. If you want to
> see the exact error - run with strict.
Thanks, I wasn't aware there is a strict version I should be using.
Good to know for the future.
Not that I can reproduce, I'll fix and send send v3. Sorry about the mess.
>
> Best regards,
> Krzysztof
>