The debugfs_create_dir() function returns error pointers,
it never returns NULL. Most incorrect error checks were fixed,
but the one in bnad_debugfs_init() was forgotten.
Fix the remaining error check.
Signed-off-by: Wang Ming <[email protected]>
Fixes: 7afc5dbde091 ("bna: Add debugfs interface.")
---
drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
index 04ad0f2b9677..678a3668a041 100644
--- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
+++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
@@ -512,7 +512,7 @@ bnad_debugfs_init(struct bnad *bnad)
if (!bnad->port_debugfs_root) {
bnad->port_debugfs_root =
debugfs_create_dir(name, bna_debugfs_root);
- if (!bnad->port_debugfs_root) {
+ if (IS_ERR(bnad->port_debugfs_root)) {
netdev_warn(bnad->netdev,
"debugfs root dir creation failed\n");
return;
--
2.25.1
Hi--
On 7/12/23 22:38, Wang Ming wrote:
> The debugfs_create_dir() function returns error pointers,
> it never returns NULL. Most incorrect error checks were fixed,
> but the one in bnad_debugfs_init() was forgotten.
>
> Fix the remaining error check.
>
> Signed-off-by: Wang Ming <[email protected]>
>
> Fixes: 7afc5dbde091 ("bna: Add debugfs interface.")
Comment from fs/debugfs/inode.c:
* NOTE: it's expected that most callers should _ignore_ the errors returned
* by this function. Other debugfs functions handle the fact that the "dentry"
* passed to them could be an error and they don't crash in that case.
* Drivers should generally work fine even if debugfs fails to init anyway.
so no, drivers should not usually care about debugfs function call results.
Is there some special case here?
> ---
> drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> index 04ad0f2b9677..678a3668a041 100644
> --- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> +++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> @@ -512,7 +512,7 @@ bnad_debugfs_init(struct bnad *bnad)
> if (!bnad->port_debugfs_root) {
> bnad->port_debugfs_root =
> debugfs_create_dir(name, bna_debugfs_root);
> - if (!bnad->port_debugfs_root) {
> + if (IS_ERR(bnad->port_debugfs_root)) {
> netdev_warn(bnad->netdev,
> "debugfs root dir creation failed\n");
> return;
--
~Randy
Ok, so I think we should delete the check operation. What do you think? If it is consistent, I will submit it again
: )
Ming
-----邮件原件-----
发件人: Randy Dunlap <[email protected]>
发送时间: 2023年7月13日 13:50
收件人: 王明-软件底层技术部 <[email protected]>; Rasesh Mody <[email protected]>; Sudarsana Kalluru <[email protected]>; [email protected]; David S. Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Krishna Gudipati <[email protected]>; [email protected]; [email protected]
抄送: opensource.kernel <[email protected]>
主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir()
[Some people who received this message don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
Hi--
On 7/12/23 22:38, Wang Ming wrote:
> The debugfs_create_dir() function returns error pointers, it never
> returns NULL. Most incorrect error checks were fixed, but the one in
> bnad_debugfs_init() was forgotten.
>
> Fix the remaining error check.
>
> Signed-off-by: Wang Ming <[email protected]>
>
> Fixes: 7afc5dbde091 ("bna: Add debugfs interface.")
Comment from fs/debugfs/inode.c:
* NOTE: it's expected that most callers should _ignore_ the errors returned
* by this function. Other debugfs functions handle the fact that the "dentry"
* passed to them could be an error and they don't crash in that case.
* Drivers should generally work fine even if debugfs fails to init anyway.
so no, drivers should not usually care about debugfs function call results.
Is there some special case here?
> ---
> drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +-
> 1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> index 04ad0f2b9677..678a3668a041 100644
> --- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> +++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
> @@ -512,7 +512,7 @@ bnad_debugfs_init(struct bnad *bnad)
> if (!bnad->port_debugfs_root) {
> bnad->port_debugfs_root =
> debugfs_create_dir(name, bna_debugfs_root);
> - if (!bnad->port_debugfs_root) {
> + if (IS_ERR(bnad->port_debugfs_root)) {
> netdev_warn(bnad->netdev,
> "debugfs root dir creation failed\n");
> return;
--
~Randy
On 7/13/23 02:05, 王明-软件底层技术部 wrote:
> Ok, so I think we should delete the check operation. What do you think? If it is consistent, I will submit it again
> : )
Yes, that's the idea. Thanks.
> Ming
> -----邮件原件-----
> 发件人: Randy Dunlap <[email protected]>
> 发送时间: 2023年7月13日 13:50
> 收件人: 王明-软件底层技术部 <[email protected]>; Rasesh Mody <[email protected]>; Sudarsana Kalluru <[email protected]>; [email protected]; David S. Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Krishna Gudipati <[email protected]>; [email protected]; [email protected]
> 抄送: opensource.kernel <[email protected]>
> 主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir()
>
> [Some people who received this message don't often get email from [email protected]. Learn why this is important at https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi--
>
> On 7/12/23 22:38, Wang Ming wrote:
>> The debugfs_create_dir() function returns error pointers, it never
>> returns NULL. Most incorrect error checks were fixed, but the one in
>> bnad_debugfs_init() was forgotten.
>>
>> Fix the remaining error check.
>>
>> Signed-off-by: Wang Ming <[email protected]>
>>
>> Fixes: 7afc5dbde091 ("bna: Add debugfs interface.")
>
> Comment from fs/debugfs/inode.c:
>
> * NOTE: it's expected that most callers should _ignore_ the errors returned
> * by this function. Other debugfs functions handle the fact that the "dentry"
> * passed to them could be an error and they don't crash in that case.
> * Drivers should generally work fine even if debugfs fails to init anyway.
>
> so no, drivers should not usually care about debugfs function call results.
> Is there some special case here?
>
>> ---
>> drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> index 04ad0f2b9677..678a3668a041 100644
>> --- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> +++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> @@ -512,7 +512,7 @@ bnad_debugfs_init(struct bnad *bnad)
>> if (!bnad->port_debugfs_root) {
>> bnad->port_debugfs_root =
>> debugfs_create_dir(name, bna_debugfs_root);
>> - if (!bnad->port_debugfs_root) {
>> + if (IS_ERR(bnad->port_debugfs_root)) {
>> netdev_warn(bnad->netdev,
>> "debugfs root dir creation failed\n");
>> return;
>
> --
> ~Randy
--
~Randy
Yes, I submitted the new version yesterday.
Regards
Wang Ming
-----邮件原件-----
发件人: Randy Dunlap <[email protected]>
发送时间: 2023年7月13日 22:34
收件人: 王明-软件底层技术部 <[email protected]>
抄送: Rasesh Mody <[email protected]>; Sudarsana Kalluru <[email protected]>; [email protected]; David S. Miller <[email protected]>; Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>; Paolo Abeni <[email protected]>; Krishna Gudipati <[email protected]>; [email protected]; LKML <[email protected]>
主题: Re: 回复: [PATCH net v1] bna:Fix error checking for debugfs_create_dir()
On 7/13/23 02:05, 王明-软件底层技术部 wrote:
> Ok, so I think we should delete the check operation. What do you
> think? If it is consistent, I will submit it again
> : )
Yes, that's the idea. Thanks.
> Ming
> -----邮件原件-----
> 发件人: Randy Dunlap <[email protected]>
> 发送时间: 2023年7月13日 13:50
> 收件人: 王明-软件底层技术部 <[email protected]>; Rasesh Mody <[email protected]>;
> Sudarsana Kalluru <[email protected]>;
> [email protected]; David S. Miller <[email protected]>;
> Eric Dumazet <[email protected]>; Jakub Kicinski <[email protected]>;
> Paolo Abeni <[email protected]>; Krishna Gudipati
> <[email protected]>; [email protected];
> [email protected]
> 抄送: opensource.kernel <[email protected]>
> 主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir()
>
> [Some people who received this message don't often get email from
> [email protected]. Learn why this is important at
> https://aka.ms/LearnAboutSenderIdentification ]
>
> Hi--
>
> On 7/12/23 22:38, Wang Ming wrote:
>> The debugfs_create_dir() function returns error pointers, it never
>> returns NULL. Most incorrect error checks were fixed, but the one in
>> bnad_debugfs_init() was forgotten.
>>
>> Fix the remaining error check.
>>
>> Signed-off-by: Wang Ming <[email protected]>
>>
>> Fixes: 7afc5dbde091 ("bna: Add debugfs interface.")
>
> Comment from fs/debugfs/inode.c:
>
> * NOTE: it's expected that most callers should _ignore_ the errors
> returned
> * by this function. Other debugfs functions handle the fact that the "dentry"
> * passed to them could be an error and they don't crash in that case.
> * Drivers should generally work fine even if debugfs fails to init anyway.
>
> so no, drivers should not usually care about debugfs function call results.
> Is there some special case here?
>
>> ---
>> drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> index 04ad0f2b9677..678a3668a041 100644
>> --- a/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> +++ b/drivers/net/ethernet/brocade/bna/bnad_debugfs.c
>> @@ -512,7 +512,7 @@ bnad_debugfs_init(struct bnad *bnad)
>> if (!bnad->port_debugfs_root) {
>> bnad->port_debugfs_root =
>> debugfs_create_dir(name, bna_debugfs_root);
>> - if (!bnad->port_debugfs_root) {
>> + if (IS_ERR(bnad->port_debugfs_root)) {
>> netdev_warn(bnad->netdev,
>> "debugfs root dir creation failed\n");
>> return;
>
> --
> ~Randy
--
~Randy