diff mbox series

[v1] drivers: edac: zynqmp_edac: Updates return value check

Message ID 20230806130514.159102-1-atulpant.linux@gmail.com (mailing list archive)
State New, archived
Headers show
Series [v1] drivers: edac: zynqmp_edac: Updates return value check | expand

Commit Message

Atul Kumar Pant Aug. 6, 2023, 1:05 p.m. UTC
Updating the check of return value from edac_debugfs_create_dir
to use IS_ERR.

Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
---
 drivers/edac/zynqmp_edac.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Greg Kroah-Hartman Aug. 6, 2023, 1:37 p.m. UTC | #1
On Sun, Aug 06, 2023 at 06:35:14PM +0530, Atul Kumar Pant wrote:
> Updating the check of return value from edac_debugfs_create_dir
> to use IS_ERR.
> 
> Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> ---
>  drivers/edac/zynqmp_edac.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/edac/zynqmp_edac.c b/drivers/edac/zynqmp_edac.c
> index ac7d1e0b324c..cefbbafb945e 100644
> --- a/drivers/edac/zynqmp_edac.c
> +++ b/drivers/edac/zynqmp_edac.c
> @@ -351,7 +351,7 @@ static void setup_debugfs(struct edac_device_ctl_info *edac_dev)
>  	struct edac_priv *priv = edac_dev->pvt_info;
>  
>  	priv->debugfs_dir = edac_debugfs_create_dir("ocm");
> -	if (!priv->debugfs_dir)
> +	if (IS_ERR(priv->debugfs_dir))
>  		return;

Again, not correct, sorry.  Please do not make these types of changes.

Why do you feel this is needed at all?

thanks,

greg k-h
Atul Kumar Pant Aug. 7, 2023, 2:31 a.m. UTC | #2
On Sun, Aug 06, 2023 at 03:37:01PM +0200, Greg KH wrote:
> On Sun, Aug 06, 2023 at 06:35:14PM +0530, Atul Kumar Pant wrote:
> > Updating the check of return value from edac_debugfs_create_dir
> > to use IS_ERR.
> > 
> > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > ---
> >  drivers/edac/zynqmp_edac.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/drivers/edac/zynqmp_edac.c b/drivers/edac/zynqmp_edac.c
> > index ac7d1e0b324c..cefbbafb945e 100644
> > --- a/drivers/edac/zynqmp_edac.c
> > +++ b/drivers/edac/zynqmp_edac.c
> > @@ -351,7 +351,7 @@ static void setup_debugfs(struct edac_device_ctl_info *edac_dev)
> >  	struct edac_priv *priv = edac_dev->pvt_info;
> >  
> >  	priv->debugfs_dir = edac_debugfs_create_dir("ocm");
> > -	if (!priv->debugfs_dir)
> > +	if (IS_ERR(priv->debugfs_dir))
> >  		return;
> 
> Again, not correct, sorry.  Please do not make these types of changes.
> 
> Why do you feel this is needed at all?
> 
    edac_debugfs_create_dir uses debugfs_create_dir that return ERR_PTR.
    Hence to check the return value by this function I changed null
    comparison with IS_ERR.

> thanks,
> 
> greg k-h
Greg Kroah-Hartman Aug. 7, 2023, 6:01 a.m. UTC | #3
On Mon, Aug 07, 2023 at 08:01:04AM +0530, Atul Kumar Pant wrote:
> On Sun, Aug 06, 2023 at 03:37:01PM +0200, Greg KH wrote:
> > On Sun, Aug 06, 2023 at 06:35:14PM +0530, Atul Kumar Pant wrote:
> > > Updating the check of return value from edac_debugfs_create_dir
> > > to use IS_ERR.
> > > 
> > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > > ---
> > >  drivers/edac/zynqmp_edac.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > 
> > > diff --git a/drivers/edac/zynqmp_edac.c b/drivers/edac/zynqmp_edac.c
> > > index ac7d1e0b324c..cefbbafb945e 100644
> > > --- a/drivers/edac/zynqmp_edac.c
> > > +++ b/drivers/edac/zynqmp_edac.c
> > > @@ -351,7 +351,7 @@ static void setup_debugfs(struct edac_device_ctl_info *edac_dev)
> > >  	struct edac_priv *priv = edac_dev->pvt_info;
> > >  
> > >  	priv->debugfs_dir = edac_debugfs_create_dir("ocm");
> > > -	if (!priv->debugfs_dir)
> > > +	if (IS_ERR(priv->debugfs_dir))
> > >  		return;
> > 
> > Again, not correct, sorry.  Please do not make these types of changes.
> > 
> > Why do you feel this is needed at all?
> > 
>     edac_debugfs_create_dir uses debugfs_create_dir that return ERR_PTR.
>     Hence to check the return value by this function I changed null
>     comparison with IS_ERR.

But that's not needed at all, right?

As this check has not ever really worked, why check it at all?

greg k-h
Atul Kumar Pant Aug. 15, 2023, 8:35 p.m. UTC | #4
On Mon, Aug 07, 2023 at 08:01:56AM +0200, Greg KH wrote:
> On Mon, Aug 07, 2023 at 08:01:04AM +0530, Atul Kumar Pant wrote:
> > On Sun, Aug 06, 2023 at 03:37:01PM +0200, Greg KH wrote:
> > > On Sun, Aug 06, 2023 at 06:35:14PM +0530, Atul Kumar Pant wrote:
> > > > Updating the check of return value from edac_debugfs_create_dir
> > > > to use IS_ERR.
> > > > 
> > > > Signed-off-by: Atul Kumar Pant <atulpant.linux@gmail.com>
> > > > ---
> > > >  drivers/edac/zynqmp_edac.c | 2 +-
> > > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > > > 
> > > > diff --git a/drivers/edac/zynqmp_edac.c b/drivers/edac/zynqmp_edac.c
> > > > index ac7d1e0b324c..cefbbafb945e 100644
> > > > --- a/drivers/edac/zynqmp_edac.c
> > > > +++ b/drivers/edac/zynqmp_edac.c
> > > > @@ -351,7 +351,7 @@ static void setup_debugfs(struct edac_device_ctl_info *edac_dev)
> > > >  	struct edac_priv *priv = edac_dev->pvt_info;
> > > >  
> > > >  	priv->debugfs_dir = edac_debugfs_create_dir("ocm");
> > > > -	if (!priv->debugfs_dir)
> > > > +	if (IS_ERR(priv->debugfs_dir))
> > > >  		return;
> > > 
> > > Again, not correct, sorry.  Please do not make these types of changes.
> > > 
> > > Why do you feel this is needed at all?
> > > 
> >     edac_debugfs_create_dir uses debugfs_create_dir that return ERR_PTR.
> >     Hence to check the return value by this function I changed null
> >     comparison with IS_ERR.
> 
> But that's not needed at all, right?
> 
> As this check has not ever really worked, why check it at all?

	I'll fix this in a new patch
> 
> greg k-h
diff mbox series

Patch

diff --git a/drivers/edac/zynqmp_edac.c b/drivers/edac/zynqmp_edac.c
index ac7d1e0b324c..cefbbafb945e 100644
--- a/drivers/edac/zynqmp_edac.c
+++ b/drivers/edac/zynqmp_edac.c
@@ -351,7 +351,7 @@  static void setup_debugfs(struct edac_device_ctl_info *edac_dev)
 	struct edac_priv *priv = edac_dev->pvt_info;
 
 	priv->debugfs_dir = edac_debugfs_create_dir("ocm");
-	if (!priv->debugfs_dir)
+	if (IS_ERR(priv->debugfs_dir))
 		return;
 
 	edac_debugfs_create_x32("inject_fault_count", 0644, priv->debugfs_dir,