On Fri, Sep 13, 2024 at 06:19:20PM +0800, Herbert Xu wrote:
> On Tue, Sep 03, 2024 at 10:42:30PM +0800, Li Zetao wrote:
> > Since the debugfs_create_dir() never returns a null pointer, checking
> > the return value for a null pointer is redundant, and using IS_ERR is
> > safe enough.
> >
> > Signed-off-by: Li Zetao <lizetao1@xxxxxxxxxx>
> > ---
> > drivers/crypto/intel/qat/qat_common/adf_dbgfs.c | 2 +-
> > 1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/drivers/crypto/intel/qat/qat_common/adf_dbgfs.c b/drivers/crypto/intel/qat/qat_common/adf_dbgfs.c
> > index c42f5c25aabd..ec2c712b9006 100644
> > --- a/drivers/crypto/intel/qat/qat_common/adf_dbgfs.c
> > +++ b/drivers/crypto/intel/qat/qat_common/adf_dbgfs.c
> > @@ -30,7 +30,7 @@ void adf_dbgfs_init(struct adf_accel_dev *accel_dev)
> > pci_name(accel_dev->accel_pci_dev.pci_dev));
> >
> > ret = debugfs_create_dir(name, NULL);
> > - if (IS_ERR_OR_NULL(ret))
> > + if (IS_ERR(ret))
> > return;
>
> There is no point in creating patches like this. It doesn't
> make the code better at all. IS_ERR_OR_NULL usually compiles
> to a single branch just like IS_ERR.
>
> However, I have to say that this code is actually buggy. Surely
> this function should be passing the error back up so that it does
> not try to create anything under the non-existant dbgfs directory?
As I understand it, there is no need to check the return value of
debugfs_create_*() functions. See f0fcf9ade46a ("crypto: qat - no need to check
return value of debugfs_create functions"), where all checks after the
debugfs_create_*() were removed.
In this particular case, the check is present only to avoid attempting to
create attributes if the directory is missing, since we know such an
attempt will fail.
Regards,
--
Giovanni
