| Message ID | 20230713053823.14898-1-machel@vivo.com (mailing list archive) |
|---|---|
| State | Changes Requested |
| Delegated to: | Netdev Maintainers |
| Headers | show |
| Series | [net,v1] bna:Fix error checking for debugfs_create_dir() | expand |
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 <machel@vivo.com> > > 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;
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 <rdunlap@infradead.org> 发送时间: 2023年7月13日 13:50 收件人: 王明-软件底层技术部 <machel@vivo.com>; Rasesh Mody <rmody@marvell.com>; Sudarsana Kalluru <skalluru@marvell.com>; GR-Linux-NIC-Dev@marvell.com; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Krishna Gudipati <kgudipat@brocade.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org 抄送: opensource.kernel <opensource.kernel@vivo.com> 主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir() [Some people who received this message don't often get email from rdunlap@infradead.org. 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 <machel@vivo.com> > > 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 <rdunlap@infradead.org> > 发送时间: 2023年7月13日 13:50 > 收件人: 王明-软件底层技术部 <machel@vivo.com>; Rasesh Mody <rmody@marvell.com>; Sudarsana Kalluru <skalluru@marvell.com>; GR-Linux-NIC-Dev@marvell.com; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Krishna Gudipati <kgudipat@brocade.com>; netdev@vger.kernel.org; linux-kernel@vger.kernel.org > 抄送: opensource.kernel <opensource.kernel@vivo.com> > 主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir() > > [Some people who received this message don't often get email from rdunlap@infradead.org. 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 <machel@vivo.com> >> >> 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
Yes, I submitted the new version yesterday. Regards Wang Ming -----邮件原件----- 发件人: Randy Dunlap <rdunlap@infradead.org> 发送时间: 2023年7月13日 22:34 收件人: 王明-软件底层技术部 <machel@vivo.com> 抄送: Rasesh Mody <rmody@marvell.com>; Sudarsana Kalluru <skalluru@marvell.com>; GR-Linux-NIC-Dev@marvell.com; David S. Miller <davem@davemloft.net>; Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; Paolo Abeni <pabeni@redhat.com>; Krishna Gudipati <kgudipat@brocade.com>; netdev@vger.kernel.org; LKML <linux-kernel@vger.kernel.org> 主题: 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 <rdunlap@infradead.org> > 发送时间: 2023年7月13日 13:50 > 收件人: 王明-软件底层技术部 <machel@vivo.com>; Rasesh Mody <rmody@marvell.com>; > Sudarsana Kalluru <skalluru@marvell.com>; > GR-Linux-NIC-Dev@marvell.com; David S. Miller <davem@davemloft.net>; > Eric Dumazet <edumazet@google.com>; Jakub Kicinski <kuba@kernel.org>; > Paolo Abeni <pabeni@redhat.com>; Krishna Gudipati > <kgudipat@brocade.com>; netdev@vger.kernel.org; > linux-kernel@vger.kernel.org > 抄送: opensource.kernel <opensource.kernel@vivo.com> > 主题: Re: [PATCH net v1] bna:Fix error checking for debugfs_create_dir() > > [Some people who received this message don't often get email from > rdunlap@infradead.org. 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 <machel@vivo.com> >> >> 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
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;
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 <machel@vivo.com> Fixes: 7afc5dbde091 ("bna: Add debugfs interface.") --- drivers/net/ethernet/brocade/bna/bnad_debugfs.c | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)