Groups | Search | Server Info | Keyboard shortcuts | Login | Register [http] [https] [nntp] [nntps]

Groups > linux.kernel > #1401152 > unrolled thread

[PATCH 0/2] mfd: twl-core: Fine-tuning for add_numbered_child()

Back to article view | Back to linux.kernel

This discussion starts older than the indexed window; earlier articles aren't shown. The article labeled Started by below is the oldest one visible, not the original post.

Contents

  [PATCH 0/2] mfd: twl-core: Fine-tuning for add_numbered_child() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-05-16 04:10 +0200
    [PATCH 1/2] mfd: twl-core: Return directly after a failed  platform_device_alloc() in add_numbered_child() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-05-16 08:30 +0200
      Re: [PATCH 1/2] mfd: twl-core: Return directly after a failed  platform_device_alloc() in add_numbered_child() Julia Lawall <julia.lawall@lip6.fr> - 2016-05-16 09:00 +0200
        Re: mfd: twl-core: Return directly after a failed  platform_device_alloc() in add_numbered_child() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-05-16 10:00 +0200
          Re: mfd: twl-core: Return directly after a failed platform_device_alloc()  in add_numbered_child() Julia Lawall <julia.lawall@lip6.fr> - 2016-05-16 10:10 +0200
            Re: mfd: twl-core: Return directly after a failed  platform_device_alloc() in add_numbered_child() Lee Jones <lee.jones@linaro.org> - 2016-05-17 08:10 +0200
              Re: mfd: twl-core: Return directly after a failed  platform_device_alloc() in add_numbered_child() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-05-17 16:20 +0200
    [PATCH 2/2] mfd: twl-core: Refactoring for add_numbered_child() SF Markus Elfring <elfring@users.sourceforge.net> - 2016-05-16 08:30 +0200

#1401152 — [PATCH 0/2] mfd: twl-core: Fine-tuning for add_numbered_child()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:55:30 +0200

A few update suggestions were taken into account
from static source code analysis.

Markus Elfring (2):
  Return directly after a failed platform_device_alloc() in add_numbered_child()
  Refactoring for add_numbered_child()

 drivers/mfd/twl-core.c | 24 +++++++++++-------------
 1 file changed, 11 insertions(+), 13 deletions(-)

-- 
2.8.2

[toc] | [next] | [standalone]

#1401323 — [PATCH 1/2] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:20:28 +0200

The platform_device_put() function was called in one case by the
add_numbered_child() function during error handling even if the passed
variable "pdev" contained a null pointer.

* Change an error message.

* Return directly in this case.

This issue was detected by using the Coccinelle software.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/twl-core.c | 7 ++++---
 1 file changed, 4 insertions(+), 3 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index 831696e..dc34e69 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -623,9 +623,10 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 
 	pdev = platform_device_alloc(name, num);
 	if (!pdev) {
-		dev_dbg(&twl->client->dev, "can't alloc dev\n");
-		status = -ENOMEM;
-		goto err;
+		dev_err(&twl->client->dev,
+			"Allocation failed for device: %s\n",
+			name);
+		return ERR_PTR(-ENOMEM);
 	}
 
 	pdev->dev.parent = &twl->client->dev;
-- 
2.8.2

[toc] | [prev] | [next] | [standalone]

#1401329 — Re: [PATCH 1/2] mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

On Mon, 16 May 2016, SF Markus Elfring wrote:

> From: Markus Elfring <elfring@users.sourceforge.net>
> Date: Sun, 15 May 2016 19:20:28 +0200
> 
> The platform_device_put() function was called in one case by the
> add_numbered_child() function during error handling even if the passed
> variable "pdev" contained a null pointer.
> 
> * Change an error message.

Why?  Is dev_err needed?  Doesn't it already print out the device name?

In any case, the only source of failure is failure of a kzalloc in 
platform_device_alloc, which means that a complete backtrace would be 
generated, so it is not clear that any message is needed at all.

julia

> 
> * Return directly in this case.
> 
> This issue was detected by using the Coccinelle software.
> 
> Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
> ---
>  drivers/mfd/twl-core.c | 7 ++++---
>  1 file changed, 4 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
> index 831696e..dc34e69 100644
> --- a/drivers/mfd/twl-core.c
> +++ b/drivers/mfd/twl-core.c
> @@ -623,9 +623,10 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
>  
>  	pdev = platform_device_alloc(name, num);
>  	if (!pdev) {
> -		dev_dbg(&twl->client->dev, "can't alloc dev\n");
> -		status = -ENOMEM;
> -		goto err;
> +		dev_err(&twl->client->dev,
> +			"Allocation failed for device: %s\n",
> +			name);
> +		return ERR_PTR(-ENOMEM);
>  	}
>  
>  	pdev->dev.parent = &twl->client->dev;
> -- 
> 2.8.2
> 
> --
> To unsubscribe from this list: send the line "unsubscribe kernel-janitors" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

[toc] | [prev] | [next] | [standalone]

#1401343 — Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

>> * Change an error message.
> 
> Why?  Is dev_err needed?

I interpreted Lee's response in this way.
https://lkml.org/lkml/2016/1/11/104

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1401347 — Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

On Mon, 16 May 2016, SF Markus Elfring wrote:

> >> * Change an error message.
> > 
> > Why?  Is dev_err needed?
> 
> I interpreted Lee's response in this way.
> https://lkml.org/lkml/2016/1/11/104

OK.  He didn't ask for the message to be changed though.  It's a bit 
unfortunate that it now takes up multiple lines.  And I believe it also 
prints redundant information.  Perhaps he will have some further thoughts 
on the matter.

julia

[toc] | [prev] | [next] | [standalone]

#1402123 — Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

On Mon, 16 May 2016, Julia Lawall wrote:

> On Mon, 16 May 2016, SF Markus Elfring wrote:
> 
> > >> * Change an error message.
> > > 
> > > Why?  Is dev_err needed?
> > 
> > I interpreted Lee's response in this way.
> > https://lkml.org/lkml/2016/1/11/104
> 
> OK.  He didn't ask for the message to be changed though.  It's a bit 
> unfortunate that it now takes up multiple lines.  And I believe it also 
> prints redundant information.  Perhaps he will have some further thoughts 
> on the matter.

Yes, Julia is right.  We normally don't print anything for OOM
errors since Linux reports on them already.

Please remove the print altogether.

-- 
Lee Jones
Linaro STMicroelectronics Landing Team Lead
Linaro.org │ Open source software for ARM SoCs
Follow Linaro: Facebook | Twitter | Blog

[toc] | [prev] | [next] | [standalone]

#1402363 — Re: mfd: twl-core: Return directly after a failed platform_device_alloc() in add_numbered_child()

> Please remove the print altogether.

Would you like to omit any extra logging statements at more source code places?

Regards,
Markus

[toc] | [prev] | [next] | [standalone]

#1401324 — [PATCH 2/2] mfd: twl-core: Refactoring for add_numbered_child()

From: Markus Elfring <elfring@users.sourceforge.net>
Date: Sun, 15 May 2016 19:50:55 +0200

Adjust jump targets according to the Linux coding style convention.
Another check for the variable "status" can be omitted then at the end.

Signed-off-by: Markus Elfring <elfring@users.sourceforge.net>
---
 drivers/mfd/twl-core.c | 19 +++++++++----------
 1 file changed, 9 insertions(+), 10 deletions(-)

diff --git a/drivers/mfd/twl-core.c b/drivers/mfd/twl-core.c
index dc34e69..3e4f4e4 100644
--- a/drivers/mfd/twl-core.c
+++ b/drivers/mfd/twl-core.c
@@ -635,7 +635,7 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 		status = platform_device_add_data(pdev, pdata, pdata_len);
 		if (status < 0) {
 			dev_dbg(&pdev->dev, "can't add platform_data\n");
-			goto err;
+			goto put_device;
 		}
 	}
 
@@ -648,21 +648,20 @@ add_numbered_child(unsigned mod_no, const char *name, int num,
 		status = platform_device_add_resources(pdev, r, irq1 ? 2 : 1);
 		if (status < 0) {
 			dev_dbg(&pdev->dev, "can't add irqs\n");
-			goto err;
+			goto put_device;
 		}
 	}
 
 	status = platform_device_add(pdev);
-	if (status == 0)
-		device_init_wakeup(&pdev->dev, can_wakeup);
+	if (status)
+		goto put_device;
 
-err:
-	if (status < 0) {
-		platform_device_put(pdev);
-		dev_err(&twl->client->dev, "can't add %s dev\n", name);
-		return ERR_PTR(status);
-	}
+	device_init_wakeup(&pdev->dev, can_wakeup);
 	return &pdev->dev;
+put_device:
+	platform_device_put(pdev);
+	dev_err(&twl->client->dev, "can't add %s dev\n", name);
+	return ERR_PTR(status);
 }
 
 static inline struct device *add_child(unsigned mod_no, const char *name,
-- 
2.8.2

[toc] | [prev] | [standalone]

Back to top | Article view | linux.kernel

csiph-web