| 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 |
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
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
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
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 --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,
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(-)