It was modified to increase readability.
Changes from v4:
added null checking of edev on each function.
Chages from v3:
removed possibility of kfree(NULL).
Chages from v2:
resolved possible memory leak of dev->cables.
Changes from v1:
added return value handling.
Bumwoo Lee (3):
extcon: Add extcon_alloc_cables to simplify extcon register function
extcon: Add extcon_alloc_muex to simplify extcon register function
extcon: Add extcon_alloc_groups to simplify extcon register function
drivers/extcon/extcon.c | 281 +++++++++++++++++++++++-----------------
1 file changed, 165 insertions(+), 116 deletions(-)
--
2.35.1
The mutual exclusive part is functionalized from extcon_dev_register.
Signed-off-by: Bumwoo Lee <[email protected]>
---
drivers/extcon/extcon.c | 109 ++++++++++++++++++++++------------------
1 file changed, 61 insertions(+), 48 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 402d674e2896..86290afa35fb 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1130,6 +1130,63 @@ static int extcon_alloc_cables(struct extcon_dev *edev)
return 0;
}
+/**
+ * extcon_alloc_muex() - alloc the mutual exclusive for extcon device
+ * @edev: extcon device
+ *
+ * Returns 0 if success or error number if fail.
+ */
+static int extcon_alloc_muex(struct extcon_dev *edev)
+{
+ char *name;
+ int index;
+
+ if (!edev)
+ return -EINVAL;
+
+ if (!(edev->max_supported && edev->mutually_exclusive))
+ return 0;
+
+ /* Count the size of mutually_exclusive array */
+ for (index = 0; edev->mutually_exclusive[index]; index++)
+ ;
+
+ edev->attrs_muex = kcalloc(index + 1,
+ sizeof(struct attribute *),
+ GFP_KERNEL);
+ if (!edev->attrs_muex)
+ return -ENOMEM;
+
+ edev->d_attrs_muex = kcalloc(index,
+ sizeof(struct device_attribute),
+ GFP_KERNEL);
+ if (!edev->d_attrs_muex) {
+ kfree(edev->attrs_muex);
+ return -ENOMEM;
+ }
+
+ for (index = 0; edev->mutually_exclusive[index]; index++) {
+ name = kasprintf(GFP_KERNEL, "0x%x",
+ edev->mutually_exclusive[index]);
+ if (!name) {
+ for (index--; index >= 0; index--)
+ kfree(edev->d_attrs_muex[index].attr.name);
+
+ kfree(edev->d_attrs_muex);
+ kfree(edev->attrs_muex);
+ return -ENOMEM;
+ }
+ sysfs_attr_init(&edev->d_attrs_muex[index].attr);
+ edev->d_attrs_muex[index].attr.name = name;
+ edev->d_attrs_muex[index].attr.mode = 0000;
+ edev->attrs_muex[index] = &edev->d_attrs_muex[index].attr;
+ }
+ edev->attr_g_muex.name = muex_name;
+ edev->attr_g_muex.attrs = edev->attrs_muex;
+
+ return 0;
+}
+
/**
* extcon_dev_register() - Register an new extcon device
* @edev: the extcon device to be registered
@@ -1181,53 +1238,9 @@ int extcon_dev_register(struct extcon_dev *edev)
if (ret < 0)
goto err_alloc_cables;
- if (edev->max_supported && edev->mutually_exclusive) {
- char *name;
-
- /* Count the size of mutually_exclusive array */
- for (index = 0; edev->mutually_exclusive[index]; index++)
- ;
-
- edev->attrs_muex = kcalloc(index + 1,
- sizeof(struct attribute *),
- GFP_KERNEL);
- if (!edev->attrs_muex) {
- ret = -ENOMEM;
- goto err_muex;
- }
-
- edev->d_attrs_muex = kcalloc(index,
- sizeof(struct device_attribute),
- GFP_KERNEL);
- if (!edev->d_attrs_muex) {
- ret = -ENOMEM;
- kfree(edev->attrs_muex);
- goto err_muex;
- }
-
- for (index = 0; edev->mutually_exclusive[index]; index++) {
- name = kasprintf(GFP_KERNEL, "0x%x",
- edev->mutually_exclusive[index]);
- if (!name) {
- for (index--; index >= 0; index--) {
- kfree(edev->d_attrs_muex[index].attr.
- name);
- }
- kfree(edev->d_attrs_muex);
- kfree(edev->attrs_muex);
- ret = -ENOMEM;
- goto err_muex;
- }
- sysfs_attr_init(&edev->d_attrs_muex[index].attr);
- edev->d_attrs_muex[index].attr.name = name;
- edev->d_attrs_muex[index].attr.mode = 0000;
- edev->attrs_muex[index] = &edev->d_attrs_muex[index]
- .attr;
- }
- edev->attr_g_muex.name = muex_name;
- edev->attr_g_muex.attrs = edev->attrs_muex;
-
- }
+ ret = extcon_alloc_muex(edev);
+ if (ret < 0)
+ goto err_alloc_muex;
if (edev->max_supported) {
edev->extcon_dev_type.groups =
@@ -1295,7 +1308,7 @@ int extcon_dev_register(struct extcon_dev *edev)
kfree(edev->d_attrs_muex);
kfree(edev->attrs_muex);
}
-err_muex:
+err_alloc_muex:
for (index = 0; index < edev->max_supported; index++)
kfree(edev->cables[index].attr_g.name);
if (edev->max_supported)
--
2.35.1
The alloc groups is functionalized from extcon_dev_register.
Signed-off-by: Bumwoo Lee <[email protected]>
---
drivers/extcon/extcon.c | 61 ++++++++++++++++++++++++++---------------
1 file changed, 39 insertions(+), 22 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index 86290afa35fb..26a84bed0874 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1187,6 +1187,42 @@ static int extcon_alloc_muex(struct extcon_dev *edev)
return 0;
}
+/**
+ * extcon_alloc_groups() - alloc the groups for extcon device
+ * @edev: extcon device
+ *
+ * Returns 0 if success or error number if fail.
+ */
+static int extcon_alloc_groups(struct extcon_dev *edev)
+{
+ int index;
+
+ if (!edev)
+ return -EINVAL;
+
+ if (!edev->max_supported)
+ return 0;
+
+ edev->extcon_dev_type.groups = kcalloc(edev->max_supported + 2,
+ sizeof(struct attribute_group *),
+ GFP_KERNEL);
+ if (!edev->extcon_dev_type.groups)
+ return -ENOMEM;
+
+ edev->extcon_dev_type.name = dev_name(&edev->dev);
+ edev->extcon_dev_type.release = dummy_sysfs_dev_release;
+
+ for (index = 0; index < edev->max_supported; index++)
+ edev->extcon_dev_type.groups[index] = &edev->cables[index].attr_g;
+
+ if (edev->mutually_exclusive)
+ edev->extcon_dev_type.groups[index] = &edev->attr_g_muex;
+
+ edev->dev.type = &edev->extcon_dev_type;
+
+ return 0;
+}
+
/**
* extcon_dev_register() - Register an new extcon device
* @edev: the extcon device to be registered
@@ -1242,28 +1278,9 @@ int extcon_dev_register(struct extcon_dev *edev)
if (ret < 0)
goto err_alloc_muex;
- if (edev->max_supported) {
- edev->extcon_dev_type.groups =
- kcalloc(edev->max_supported + 2,
- sizeof(struct attribute_group *),
- GFP_KERNEL);
- if (!edev->extcon_dev_type.groups) {
- ret = -ENOMEM;
- goto err_alloc_groups;
- }
-
- edev->extcon_dev_type.name = dev_name(&edev->dev);
- edev->extcon_dev_type.release = dummy_sysfs_dev_release;
-
- for (index = 0; index < edev->max_supported; index++)
- edev->extcon_dev_type.groups[index] =
- &edev->cables[index].attr_g;
- if (edev->mutually_exclusive)
- edev->extcon_dev_type.groups[index] =
- &edev->attr_g_muex;
-
- edev->dev.type = &edev->extcon_dev_type;
- }
+ ret = extcon_alloc_groups(edev);
+ if (ret < 0)
+ goto err_alloc_groups;
spin_lock_init(&edev->lock);
if (edev->max_supported) {
--
2.35.1
The cable allocation part is functionalized from extcon_dev_register.
Signed-off-by: Bumwoo Lee <[email protected]>
---
drivers/extcon/extcon.c | 111 +++++++++++++++++++++++-----------------
1 file changed, 65 insertions(+), 46 deletions(-)
diff --git a/drivers/extcon/extcon.c b/drivers/extcon/extcon.c
index adcf01132f70..402d674e2896 100644
--- a/drivers/extcon/extcon.c
+++ b/drivers/extcon/extcon.c
@@ -1070,6 +1070,66 @@ void extcon_dev_free(struct extcon_dev *edev)
}
EXPORT_SYMBOL_GPL(extcon_dev_free);
+/**
+ * extcon_alloc_cables() - alloc the cables for extcon device
+ * @edev: extcon device which has cables
+ *
+ * Returns 0 if success or error number if fail.
+ */
+static int extcon_alloc_cables(struct extcon_dev *edev)
+{
+ int index;
+ char *str;
+ struct extcon_cable *cable;
+
+ if (!edev)
+ return -EINVAL;
+
+ if (!edev->max_supported)
+ return 0;
+
+ edev->cables = kcalloc(edev->max_supported,
+ sizeof(struct extcon_cable),
+ GFP_KERNEL);
+ if (!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;
+ }
+
+ cable->edev = edev;
+ cable->cable_index = index;
+ cable->attrs[0] = &cable->attr_name.attr;
+ cable->attrs[1] = &cable->attr_state.attr;
+ cable->attrs[2] = NULL;
+ cable->attr_g.name = str;
+ cable->attr_g.attrs = cable->attrs;
+
+ sysfs_attr_init(&cable->attr_name.attr);
+ cable->attr_name.attr.name = "name";
+ cable->attr_name.attr.mode = 0444;
+ cable->attr_name.show = cable_name_show;
+
+ sysfs_attr_init(&cable->attr_state.attr);
+ cable->attr_state.attr.name = "state";
+ cable->attr_state.attr.mode = 0444;
+ cable->attr_state.show = cable_state_show;
+ }
+
+ return 0;
+}
+
/**
* extcon_dev_register() - Register an new extcon device
* @edev: the extcon device to be registered
@@ -1117,50 +1177,9 @@ int extcon_dev_register(struct extcon_dev *edev)
dev_set_name(&edev->dev, "extcon%lu",
(unsigned long)atomic_inc_return(&edev_no));
- if (edev->max_supported) {
- char *str;
- struct extcon_cable *cable;
-
- edev->cables = kcalloc(edev->max_supported,
- sizeof(struct extcon_cable),
- GFP_KERNEL);
- if (!edev->cables) {
- ret = -ENOMEM;
- goto err_sysfs_alloc;
- }
- 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);
- }
- ret = -ENOMEM;
-
- goto err_alloc_cables;
- }
-
- cable->edev = edev;
- cable->cable_index = index;
- cable->attrs[0] = &cable->attr_name.attr;
- cable->attrs[1] = &cable->attr_state.attr;
- cable->attrs[2] = NULL;
- cable->attr_g.name = str;
- cable->attr_g.attrs = cable->attrs;
-
- sysfs_attr_init(&cable->attr_name.attr);
- cable->attr_name.attr.name = "name";
- cable->attr_name.attr.mode = 0444;
- cable->attr_name.show = cable_name_show;
-
- sysfs_attr_init(&cable->attr_state.attr);
- cable->attr_state.attr.name = "state";
- cable->attr_state.attr.mode = 0444;
- cable->attr_state.show = cable_state_show;
- }
- }
+ ret = extcon_alloc_cables(edev);
+ if (ret < 0)
+ goto err_alloc_cables;
if (edev->max_supported && edev->mutually_exclusive) {
char *name;
@@ -1279,10 +1298,10 @@ int extcon_dev_register(struct extcon_dev *edev)
err_muex:
for (index = 0; index < edev->max_supported; index++)
kfree(edev->cables[index].attr_g.name);
-err_alloc_cables:
if (edev->max_supported)
kfree(edev->cables);
-err_sysfs_alloc:
+err_alloc_cables:
+
return ret;
}
EXPORT_SYMBOL_GPL(extcon_dev_register);
--
2.35.1
On 23. 3. 20. 12:19, Bumwoo Lee wrote:
> It was modified to increase readability.
>
> Changes from v4:
> added null checking of edev on each function.
>
> Chages from v3:
> removed possibility of kfree(NULL).
>
> Chages from v2:
> resolved possible memory leak of dev->cables.
>
> Changes from v1:
> added return value handling.
>
> Bumwoo Lee (3):
> extcon: Add extcon_alloc_cables to simplify extcon register function
> extcon: Add extcon_alloc_muex to simplify extcon register function
> extcon: Add extcon_alloc_groups to simplify extcon register function
>
> drivers/extcon/extcon.c | 281 +++++++++++++++++++++++-----------------
> 1 file changed, 165 insertions(+), 116 deletions(-)
>
Applied them. Thanks.
--
Best Regards,
Samsung Electronics
Chanwoo Choi