* [bug report] ath11k: add debugfs for TWT debug calls
@ 2022-03-01 7:49 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-03-01 7:49 UTC (permalink / raw)
To: john; +Cc: ath11k, linux-wireless
Hello John Crispin,
The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
from Jan 31, 2022, leads to the following Smatch static checker
warning:
drivers/net/wireless/ath/ath11k/debugfs.c:1642 ath11k_debugfs_add_interface()
warn: 'arvif->debugfs_twt' is an error pointer or valid
drivers/net/wireless/ath/ath11k/debugfs.c
1637 int ath11k_debugfs_add_interface(struct ath11k_vif *arvif)
1638 {
1639 if (arvif->vif->type == NL80211_IFTYPE_AP && !arvif->debugfs_twt) {
1640 arvif->debugfs_twt = debugfs_create_dir("twt",
1641 arvif->vif->debugfs_dir);
--> 1642 if (!arvif->debugfs_twt || IS_ERR(arvif->debugfs_twt)) {
1643 ath11k_warn(arvif->ar->ab,
1644 "failed to create directory %p\n",
1645 arvif->debugfs_twt);
The debugfs_create_dir() function never returns NULL. It's generally
not supposed to checked for errors. This code here looks like a
layering violation because it's trying to check if debugfs is already
registered. But the clean up code just unregisters on the first call.
Should it be ref counted or can the !arvif->debugfs_twt check be
removed?
Also if the user deliberately disabled debugfs then this prints an error
message.
1646 arvif->debugfs_twt = NULL;
1647 return -1;
1648 }
1649
1650 debugfs_create_file("add_dialog", 0200, arvif->debugfs_twt,
1651 arvif, &ath11k_fops_twt_add_dialog);
1652
1653 debugfs_create_file("del_dialog", 0200, arvif->debugfs_twt,
1654 arvif, &ath11k_fops_twt_del_dialog);
1655
1656 debugfs_create_file("pause_dialog", 0200, arvif->debugfs_twt,
1657 arvif, &ath11k_fops_twt_pause_dialog);
1658
1659 debugfs_create_file("resume_dialog", 0200, arvif->debugfs_twt,
1660 arvif, &ath11k_fops_twt_resume_dialog);
1661 }
1662 return 0;
1663 }
regards,
dan carpenter
^ permalink raw reply [flat|nested] 6+ messages in thread
* [bug report] ath11k: add debugfs for TWT debug calls
@ 2022-03-01 7:49 ` Dan Carpenter
0 siblings, 0 replies; 6+ messages in thread
From: Dan Carpenter @ 2022-03-01 7:49 UTC (permalink / raw)
To: john; +Cc: ath11k, linux-wireless
Hello John Crispin,
The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
from Jan 31, 2022, leads to the following Smatch static checker
warning:
drivers/net/wireless/ath/ath11k/debugfs.c:1642 ath11k_debugfs_add_interface()
warn: 'arvif->debugfs_twt' is an error pointer or valid
drivers/net/wireless/ath/ath11k/debugfs.c
1637 int ath11k_debugfs_add_interface(struct ath11k_vif *arvif)
1638 {
1639 if (arvif->vif->type == NL80211_IFTYPE_AP && !arvif->debugfs_twt) {
1640 arvif->debugfs_twt = debugfs_create_dir("twt",
1641 arvif->vif->debugfs_dir);
--> 1642 if (!arvif->debugfs_twt || IS_ERR(arvif->debugfs_twt)) {
1643 ath11k_warn(arvif->ar->ab,
1644 "failed to create directory %p\n",
1645 arvif->debugfs_twt);
The debugfs_create_dir() function never returns NULL. It's generally
not supposed to checked for errors. This code here looks like a
layering violation because it's trying to check if debugfs is already
registered. But the clean up code just unregisters on the first call.
Should it be ref counted or can the !arvif->debugfs_twt check be
removed?
Also if the user deliberately disabled debugfs then this prints an error
message.
1646 arvif->debugfs_twt = NULL;
1647 return -1;
1648 }
1649
1650 debugfs_create_file("add_dialog", 0200, arvif->debugfs_twt,
1651 arvif, &ath11k_fops_twt_add_dialog);
1652
1653 debugfs_create_file("del_dialog", 0200, arvif->debugfs_twt,
1654 arvif, &ath11k_fops_twt_del_dialog);
1655
1656 debugfs_create_file("pause_dialog", 0200, arvif->debugfs_twt,
1657 arvif, &ath11k_fops_twt_pause_dialog);
1658
1659 debugfs_create_file("resume_dialog", 0200, arvif->debugfs_twt,
1660 arvif, &ath11k_fops_twt_resume_dialog);
1661 }
1662 return 0;
1663 }
regards,
dan carpenter
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] ath11k: add debugfs for TWT debug calls
2022-03-01 7:49 ` Dan Carpenter
@ 2022-04-05 7:49 ` Kalle Valo
-1 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2022-04-05 7:49 UTC (permalink / raw)
To: Dan Carpenter, Aloka Dixit; +Cc: john, ath11k, linux-wireless
+ aloka
Dan Carpenter <dan.carpenter@oracle.com> writes:
> Hello John Crispin,
>
> The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
> from Jan 31, 2022, leads to the following Smatch static checker
> warning:
>
> drivers/net/wireless/ath/ath11k/debugfs.c:1642 ath11k_debugfs_add_interface()
> warn: 'arvif->debugfs_twt' is an error pointer or valid
>
> drivers/net/wireless/ath/ath11k/debugfs.c
> 1637 int ath11k_debugfs_add_interface(struct ath11k_vif *arvif)
> 1638 {
> 1639 if (arvif->vif->type == NL80211_IFTYPE_AP && !arvif->debugfs_twt) {
> 1640 arvif->debugfs_twt = debugfs_create_dir("twt",
> 1641 arvif->vif->debugfs_dir);
> --> 1642 if (!arvif->debugfs_twt || IS_ERR(arvif->debugfs_twt)) {
> 1643 ath11k_warn(arvif->ar->ab,
> 1644 "failed to create directory %p\n",
> 1645 arvif->debugfs_twt);
>
> The debugfs_create_dir() function never returns NULL. It's generally
> not supposed to checked for errors. This code here looks like a
> layering violation because it's trying to check if debugfs is already
> registered. But the clean up code just unregisters on the first call.
> Should it be ref counted or can the !arvif->debugfs_twt check be
> removed?
>
> Also if the user deliberately disabled debugfs then this prints an error
> message.
>
> 1646 arvif->debugfs_twt = NULL;
> 1647 return -1;
Please also use proper error values, not -1.
> 1648 }
> 1649
> 1650 debugfs_create_file("add_dialog", 0200, arvif->debugfs_twt,
> 1651 arvif, &ath11k_fops_twt_add_dialog);
> 1652
> 1653 debugfs_create_file("del_dialog", 0200, arvif->debugfs_twt,
> 1654 arvif, &ath11k_fops_twt_del_dialog);
> 1655
> 1656 debugfs_create_file("pause_dialog", 0200, arvif->debugfs_twt,
> 1657 arvif, &ath11k_fops_twt_pause_dialog);
> 1658
> 1659 debugfs_create_file("resume_dialog", 0200, arvif->debugfs_twt,
> 1660 arvif, &ath11k_fops_twt_resume_dialog);
> 1661 }
> 1662 return 0;
> 1663 }
Aloka, you submitted this patch. Please take a look and fix the issues.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] ath11k: add debugfs for TWT debug calls
@ 2022-04-05 7:49 ` Kalle Valo
0 siblings, 0 replies; 6+ messages in thread
From: Kalle Valo @ 2022-04-05 7:49 UTC (permalink / raw)
To: Dan Carpenter, Aloka Dixit; +Cc: john, ath11k, linux-wireless
+ aloka
Dan Carpenter <dan.carpenter@oracle.com> writes:
> Hello John Crispin,
>
> The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
> from Jan 31, 2022, leads to the following Smatch static checker
> warning:
>
> drivers/net/wireless/ath/ath11k/debugfs.c:1642 ath11k_debugfs_add_interface()
> warn: 'arvif->debugfs_twt' is an error pointer or valid
>
> drivers/net/wireless/ath/ath11k/debugfs.c
> 1637 int ath11k_debugfs_add_interface(struct ath11k_vif *arvif)
> 1638 {
> 1639 if (arvif->vif->type == NL80211_IFTYPE_AP && !arvif->debugfs_twt) {
> 1640 arvif->debugfs_twt = debugfs_create_dir("twt",
> 1641 arvif->vif->debugfs_dir);
> --> 1642 if (!arvif->debugfs_twt || IS_ERR(arvif->debugfs_twt)) {
> 1643 ath11k_warn(arvif->ar->ab,
> 1644 "failed to create directory %p\n",
> 1645 arvif->debugfs_twt);
>
> The debugfs_create_dir() function never returns NULL. It's generally
> not supposed to checked for errors. This code here looks like a
> layering violation because it's trying to check if debugfs is already
> registered. But the clean up code just unregisters on the first call.
> Should it be ref counted or can the !arvif->debugfs_twt check be
> removed?
>
> Also if the user deliberately disabled debugfs then this prints an error
> message.
>
> 1646 arvif->debugfs_twt = NULL;
> 1647 return -1;
Please also use proper error values, not -1.
> 1648 }
> 1649
> 1650 debugfs_create_file("add_dialog", 0200, arvif->debugfs_twt,
> 1651 arvif, &ath11k_fops_twt_add_dialog);
> 1652
> 1653 debugfs_create_file("del_dialog", 0200, arvif->debugfs_twt,
> 1654 arvif, &ath11k_fops_twt_del_dialog);
> 1655
> 1656 debugfs_create_file("pause_dialog", 0200, arvif->debugfs_twt,
> 1657 arvif, &ath11k_fops_twt_pause_dialog);
> 1658
> 1659 debugfs_create_file("resume_dialog", 0200, arvif->debugfs_twt,
> 1660 arvif, &ath11k_fops_twt_resume_dialog);
> 1661 }
> 1662 return 0;
> 1663 }
Aloka, you submitted this patch. Please take a look and fix the issues.
--
https://patchwork.kernel.org/project/linux-wireless/list/
https://wireless.wiki.kernel.org/en/developers/documentation/submittingpatches
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] ath11k: add debugfs for TWT debug calls
2022-04-05 7:49 ` Kalle Valo
@ 2022-04-07 17:32 ` Aloka Dixit
-1 siblings, 0 replies; 6+ messages in thread
From: Aloka Dixit @ 2022-04-07 17:32 UTC (permalink / raw)
To: Kalle Valo, Dan Carpenter; +Cc: john, ath11k, linux-wireless
On 4/5/2022 12:49 AM, Kalle Valo wrote:
> + aloka
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
>> Hello John Crispin,
>>
>> The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
>> from Jan 31, 2022, leads to the following Smatch static checker
>> warning:
>>
>
> Aloka, you submitted this patch. Please take a look and fix the issues.
>
Will send a fix. Thanks.
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [bug report] ath11k: add debugfs for TWT debug calls
@ 2022-04-07 17:32 ` Aloka Dixit
0 siblings, 0 replies; 6+ messages in thread
From: Aloka Dixit @ 2022-04-07 17:32 UTC (permalink / raw)
To: Kalle Valo, Dan Carpenter; +Cc: john, ath11k, linux-wireless
On 4/5/2022 12:49 AM, Kalle Valo wrote:
> + aloka
>
> Dan Carpenter <dan.carpenter@oracle.com> writes:
>
>> Hello John Crispin,
>>
>> The patch fe98a6137d03: "ath11k: add debugfs for TWT debug calls"
>> from Jan 31, 2022, leads to the following Smatch static checker
>> warning:
>>
>
> Aloka, you submitted this patch. Please take a look and fix the issues.
>
Will send a fix. Thanks.
--
ath11k mailing list
ath11k@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/ath11k
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2022-04-07 17:33 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-03-01 7:49 [bug report] ath11k: add debugfs for TWT debug calls Dan Carpenter
2022-03-01 7:49 ` Dan Carpenter
2022-04-05 7:49 ` Kalle Valo
2022-04-05 7:49 ` Kalle Valo
2022-04-07 17:32 ` Aloka Dixit
2022-04-07 17:32 ` Aloka Dixit
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.