diff mbox series

[v1,08/14] extcon: Switch to use kasprintf_strarray()

Message ID	20230322144005.40368-9-andriy.shevchenko@linux.intel.com
State	New
Headers	Received-SPF: pass (google.com: domain of linux-kernel-owner@vger.kernel.org designates 2620:137:e000::1:20 as permitted sender) client-ip=2620:137:e000::1:20; From: Andy Shevchenko <andriy.shevchenko@linux.intel.com> To: Bumwoo Lee <bw365.lee@samsung.com>, Andy Shevchenko <andriy.shevchenko@linux.intel.com>, linux-kernel@vger.kernel.org Cc: MyungJoo Ham <myungjoo.ham@samsung.com>, Chanwoo Choi <cw00.choi@samsung.com> Subject: [PATCH v1 08/14] extcon: Switch to use kasprintf_strarray() Date: Wed, 22 Mar 2023 16:39:59 +0200 Message-Id: <20230322144005.40368-9-andriy.shevchenko@linux.intel.com> In-Reply-To: <20230322144005.40368-1-andriy.shevchenko@linux.intel.com> References: <20230322144005.40368-1-andriy.shevchenko@linux.intel.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Precedence: bulk
Series	extcon: Core cleanups and documentation fixes \| [v1,00/14] extcon: Core cleanups and documentation fixes [v1,01/14] extcon: Fix kernel doc of property fields to avoid warnings [v1,02/14] extcon: Fix kernel doc of property capability fields to avoid warnings [v1,03/14] extcon: Use DECLARE_BITMAP() to declare bit arrays [v1,04/14] extcon: use sysfs_emit() to instead of sprintf() [v1,05/14] extcon: Amend kernel documentation of struct extcon_dev [v1,06/14] extcon: Allow name to be assigned outside of the framework [v1,07/14] extcon: Use unique number for the extcon device ID [v1,08/14] extcon: Switch to use kasprintf_strarray() [v1,09/14] extcon: Use device_match_of_node() helper [v1,10/14] extcon: use dev_of_node(dev) instead of dev->of_node [v1,11/14] extcon: Remove dup device name in the message and unneeded error check [v1,12/14] extcon: Use sizeof(*pointer) instead of sizeof(type) [v1,13/14] extcon: Drop unneeded assignments [v1,14/14] extcon: Use positive conditional in ternary operator

Commit Message

Andy Shevchenko March 22, 2023, 2:39 p.m. UTC

  Since we have a generic helper, switch the module to use it.
No functional change intended.

Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
---
 drivers/extcon/extcon.c | 25 +++++++++++--------------
 drivers/extcon/extcon.h |  1 +
 2 files changed, 12 insertions(+), 14 deletions(-)

Comments

Chanwoo Choi April 3, 2023, 2:58 p.m. UTC | #1

On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> Since we have a generic helper, switch the module to use it.
> No functional change intended.
> 
> Signed-off-by: Andy Shevchenko <andriy.shevchenko@linux.intel.com>
> ---
>  drivers/extcon/extcon.c | 25 +++++++++++--------------
>  drivers/extcon/extcon.h |  1 +
>  2 files changed, 12 insertions(+), 14 deletions(-)
> 
> diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
> index 0d261ec6c473..a63e7eef02fd 100644
> --- a/drivers/extcon/extcon.c
> +++ b/drivers/extcon/extcon.c
> @@ -23,6 +23,7 @@
>  #include <linux/err.h>
>  #include <linux/of.h>
>  #include <linux/slab.h>
> +#include <linux/string_helpers.h>
>  #include <linux/sysfs.h>
>  
>  #include "extcon.h"
> @@ -1104,19 +1105,17 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
>  	if (!edev->cables)
>  		return -ENOMEM;
>  
> +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> +	if (!edev->cable_names) {
> +		kfree(edev->cables);
> +		return -ENOMEM;
> +	}
> +
>  	for (index = 0; index < edev->max_supported; index++) {
>  		cable = &edev->cables[index];
>  
> -		str = kasprintf(GFP_KERNEL, "cable.%d", index);
> -		if (!str) {
> -			for (index--; index >= 0; index--) {
> -				cable = &edev->cables[index];
> -				kfree(cable->attr_g.name);
> -			}
> -
> -			kfree(edev->cables);
> -			return -ENOMEM;
> -		}
> +		str = edev->cable_names[index];
> +		strreplace(str, '-', '.');
>  
>  		cable->edev = edev;
>  		cable->cable_index = index;
> @@ -1341,8 +1340,7 @@ int extcon_dev_register(struct extcon_dev *edev)
>  		kfree(edev->attrs_muex);
>  	}
>  err_alloc_muex:
> -	for (index = 0; index < edev->max_supported; index++)
> -		kfree(edev->cables[index].attr_g.name);
> +	kfree_strarray(edev->cable_names, edev->max_supported);
>  	if (edev->max_supported)
>  		kfree(edev->cables);
>  err_alloc_cables:
> @@ -1387,8 +1385,7 @@ void extcon_dev_unregister(struct extcon_dev *edev)
>  		kfree(edev->attrs_muex);
>  	}
>  
> -	for (index = 0; index < edev->max_supported; index++)
> -		kfree(edev->cables[index].attr_g.name);
> +	kfree_strarray(edev->cable_names, edev->max_supported);
>  
>  	if (edev->max_supported) {
>  		kfree(edev->extcon_dev_type.groups);
> diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
> index 877c0860e300..5624f65ba17a 100644
> --- a/drivers/extcon/extcon.h
> +++ b/drivers/extcon/extcon.h
> @@ -58,6 +58,7 @@ struct extcon_dev {
>  	/* /sys/class/extcon/.../cable.n/... */
>  	struct device_type extcon_dev_type;
>  	struct extcon_cable *cables;
> +	char **cable_names;

The extcon cable information should be included in struct extcon_cable
in order to gather information into one point like encapsulation.

I don't prefer to add 'cable_names'.

>  
>  	/* /sys/class/extcon/.../mutually_exclusive/... */
>  	struct attribute_group attr_g_muex;

Andy Shevchenko April 5, 2023, 8:16 a.m. UTC | #2

On Mon, Apr 03, 2023 at 11:58:41PM +0900, Chanwoo Choi wrote:
> On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> > Since we have a generic helper, switch the module to use it.
> > No functional change intended.

...

> > +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> > +	if (!edev->cable_names) {
> > +		kfree(edev->cables);
> > +		return -ENOMEM;
> > +	}
> > +
> >  	for (index = 0; index < edev->max_supported; index++) {
> >  		cable = &edev->cables[index];
> >  

> > +		str = edev->cable_names[index];
> > +		strreplace(str, '-', '.');
> >  
> >  		cable->edev = edev;
> >  		cable->cable_index = index;

...

> >  	/* /sys/class/extcon/.../cable.n/... */
> >  	struct device_type extcon_dev_type;
> >  	struct extcon_cable *cables;
> > +	char **cable_names;
> 
> The extcon cable information should be included in struct extcon_cable
> in order to gather information into one point like encapsulation.
> 
> I don't prefer to add 'cable_names'.

I don't get this. The idea is to allocate the cable names in one call,
we have already API for that. The cable names are kept in the struct
extcon_cable as before. So, functionally it doesn't change anything,
it is a simple cleanup.

Andy Shevchenko April 5, 2023, 3:05 p.m. UTC | #3

On Wed, Apr 05, 2023 at 11:16:36AM +0300, Andy Shevchenko wrote:
> On Mon, Apr 03, 2023 at 11:58:41PM +0900, Chanwoo Choi wrote:
> > On 23. 3. 22. 23:39, Andy Shevchenko wrote:
> > > Since we have a generic helper, switch the module to use it.
> > > No functional change intended.

...

> > > +	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
> > > +	if (!edev->cable_names) {
> > > +		kfree(edev->cables);
> > > +		return -ENOMEM;
> > > +	}
> > > +
> > >  	for (index = 0; index < edev->max_supported; index++) {
> > >  		cable = &edev->cables[index];
> > >  
> 
> > > +		str = edev->cable_names[index];
> > > +		strreplace(str, '-', '.');
> > >  
> > >  		cable->edev = edev;
> > >  		cable->cable_index = index;
> 
> ...
> 
> > >  	/* /sys/class/extcon/.../cable.n/... */
> > >  	struct device_type extcon_dev_type;
> > >  	struct extcon_cable *cables;
> > > +	char **cable_names;
> > 
> > The extcon cable information should be included in struct extcon_cable
> > in order to gather information into one point like encapsulation.
> > 
> > I don't prefer to add 'cable_names'.
> 
> I don't get this. The idea is to allocate the cable names in one call,
> we have already API for that. The cable names are kept in the struct
> extcon_cable as before. So, functionally it doesn't change anything,
> it is a simple cleanup.

Okay, I see now your point. I can redo this a bit better I think.

diff mbox series

Patch

diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 0d261ec6c473..a63e7eef02fd 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -23,6 +23,7 @@ 
 #include <linux/err.h>
 #include <linux/of.h>
 #include <linux/slab.h>
+#include <linux/string_helpers.h>
 #include <linux/sysfs.h>
 
 #include "extcon.h"
@@ -1104,19 +1105,17 @@  static int extcon_alloc_cables(struct extcon_dev *edev)
 	if (!edev->cables)
 		return -ENOMEM;
 
+	edev->cable_names = kasprintf_strarray(GFP_KERNEL, "cable", edev->max_supported);
+	if (!edev->cable_names) {
+		kfree(edev->cables);
+		return -ENOMEM;
+	}
+
 	for (index = 0; index < edev->max_supported; index++) {
 		cable = &edev->cables[index];
 
-		str = kasprintf(GFP_KERNEL, "cable.%d", index);
-		if (!str) {
-			for (index--; index >= 0; index--) {
-				cable = &edev->cables[index];
-				kfree(cable->attr_g.name);
-			}
-
-			kfree(edev->cables);
-			return -ENOMEM;
-		}
+		str = edev->cable_names[index];
+		strreplace(str, '-', '.');
 
 		cable->edev = edev;
 		cable->cable_index = index;
@@ -1341,8 +1340,7 @@  int extcon_dev_register(struct extcon_dev *edev)
 		kfree(edev->attrs_muex);
 	}
 err_alloc_muex:
-	for (index = 0; index < edev->max_supported; index++)
-		kfree(edev->cables[index].attr_g.name);
+	kfree_strarray(edev->cable_names, edev->max_supported);
 	if (edev->max_supported)
 		kfree(edev->cables);
 err_alloc_cables:
@@ -1387,8 +1385,7 @@  void extcon_dev_unregister(struct extcon_dev *edev)
 		kfree(edev->attrs_muex);
 	}
 
-	for (index = 0; index < edev->max_supported; index++)
-		kfree(edev->cables[index].attr_g.name);
+	kfree_strarray(edev->cable_names, edev->max_supported);
 
 	if (edev->max_supported) {
 		kfree(edev->extcon_dev_type.groups);
diff --git a/drivers/extcon/extcon.h b/drivers/extcon/extcon.h
index 877c0860e300..5624f65ba17a 100644
--- a/drivers/extcon/extcon.h
+++ b/drivers/extcon/extcon.h
@@ -58,6 +58,7 @@  struct extcon_dev {
 	/* /sys/class/extcon/.../cable.n/... */
 	struct device_type extcon_dev_type;
 	struct extcon_cable *cables;
+	char **cable_names;
 
 	/* /sys/class/extcon/.../mutually_exclusive/... */
 	struct attribute_group attr_g_muex;