net: dsa: add missing kfree on remove

PATCH Superseded Neil Armstrong 2 participants 6 messages Oct 3, 2015 – Oct 6, 2015

Subsystems (3)

Networking Network drivers Networking DSA

lore

Share & tools

lore

Neil Armstrong

author

2015-10-03 14:25 UTC #0 lore raw

PATCH v2 1/5 net: dsa: add missing kfree on remove

To prevent memory leakage on unbinding, add missing kfree calls.

Signed-off-by Neil Armstrong <[email protected]>

---
 net/dsa/dsa.c | 5 ++++-
 1 file changed, 4 insertions(+), 1 deletion(-)

net/dsa/dsa.c +4 -1

index c59fa5d..12cec40 100644
@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
 	for (i = 0; i < dst->pd->nr_chips; i++) {
 		struct dsa_switch *ds = dst->ds[i];
 
-		if (ds != NULL)
+		if (ds != NULL) {
 			dsa_switch_destroy(ds);
+			kfree(ds);
+		}
 	}
 }

@@ -924,6 +926,7 @@ static int dsa_remove(struct platform_device *pdev)
 	struct dsa_switch_tree *dst = platform_get_drvdata(pdev);
 
 	dsa_remove_dst(dst);
+	kfree(dst);
 	dsa_of_remove(&pdev->dev);
 
 	return 0;

Signature · 1 line

1.9.1

Sergei Shtylyov

2015-10-03 14:33 UTC #1 lore raw

Hello.

Quoted 12 lines · Neil Armstrong

To prevent memory leakage on unbinding, add missing kfree calls.
Signed-off-by Neil Armstrong <[email protected]>
---
  net/dsa/dsa.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
net/dsa/dsa.c +0 -0
index c59fa5d..12cec40 100644

net/dsa/dsa.c static void dsa_remove_dst(struct dsa_switch_tree *dst) · line 917

Quoted 5 lines

net/dsa/dsa.c +1 -1

@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
  	for (i = 0; i < dst->pd->nr_chips; i++) {
  		struct dsa_switch *ds = dst->ds[i];
 
-		if (ds != NULL)
+		if (ds != NULL) {

Didn;t scripts/checkpatch.pl complain here? just if (ds) is preferred in
the networking code.

MBR, Sergei

Neil Armstrong

author

2015-10-03 15:20 UTC #2 lore raw

Quoted 19 lines · Sergei Shtylyov

Hello.
Quoted 15 lines · Neil Armstrong
To prevent memory leakage on unbinding, add missing kfree calls.
Signed-off-by Neil Armstrong <[email protected]>
---
  net/dsa/dsa.c | 5 ++++-
  1 file changed, 4 insertions(+), 1 deletion(-)
net/dsa/dsa.c +0 -0
index c59fa5d..12cec40 100644
@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
      for (i = 0; i < dst->pd->nr_chips; i++) {
          struct dsa_switch *ds = dst->ds[i];
 

Quoted 7 lines

net/dsa/dsa.c static void dsa_remove_dst(struct dsa_switch_tree *dst) · line 917
Quoted 2 lines
net/dsa/dsa.c +1 -1
-        if (ds != NULL)
+        if (ds != NULL) {
Didn;t scripts/checkpatch.pl complain here? just if (ds) is preferred in the networking code.
MBR, Sergei

Yes,

But I considered the cosmetic changes are not the subject of this serie.

Neil

Sergei Shtylyov

2015-10-03 21:54 UTC #3 lore raw

Quoted 22 lines · Neil Armstrong

Quoted 22 lines · Neil Armstrong
Quoted 13 lines
To prevent memory leakage on unbinding, add missing kfree calls.
Signed-off-by Neil Armstrong <[email protected]>
---
   net/dsa/dsa.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)
net/dsa/dsa.c +0 -0
index c59fa5d..12cec40 100644
net/dsa/dsa.c static void dsa_remove_dst(struct dsa_switch_tree *dst) · line 917
Quoted 5 lines
net/dsa/dsa.c +1 -1
@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
       for (i = 0; i < dst->pd->nr_chips; i++) {
           struct dsa_switch *ds = dst->ds[i];
 
-        if (ds != NULL)
+        if (ds != NULL) {
Didn;t scripts/checkpatch.pl complain here? just if (ds) is preferred in the networking code.

Quoted 5 lines

Quoted 2 lines
MBR, Sergei
Yes,
But I considered the cosmetic changes are not the subject of this serie.

Formally, all the patches should be checkpatch-clean...

Quoted 1 line

Neil

MBR, Sergei

Neil Armstrong

author

2015-10-06 07:25 UTC #4 lore raw

Quoted 28 lines · Sergei Shtylyov

Quoted 26 lines · Neil Armstrong
Quoted 17 lines · Neil Armstrong
Quoted 16 lines
To prevent memory leakage on unbinding, add missing kfree calls.
Signed-off-by Neil Armstrong <[email protected]>
---
   net/dsa/dsa.c | 5 ++++-
   1 file changed, 4 insertions(+), 1 deletion(-)
net/dsa/dsa.c +0 -0
index c59fa5d..12cec40 100644
@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
       for (i = 0; i < dst->pd->nr_chips; i++) {
           struct dsa_switch *ds = dst->ds[i];
 
Quoted 7 lines
net/dsa/dsa.c static void dsa_remove_dst(struct dsa_switch_tree *dst) · line 917
Quoted 2 lines
net/dsa/dsa.c +1 -1
-        if (ds != NULL)
+        if (ds != NULL) {
Didn;t scripts/checkpatch.pl complain here? just if (ds) is preferred in the networking code.
MBR, Sergei
Yes,

Quoted 8 lines

Quoted 1 line
But I considered the cosmetic changes are not the subject of this serie.
Formally, all the patches should be checkpatch-clean...
Quoted 1 line
Neil
MBR, Sergei

Sure,

How should I handle this case ?
A separate patch with the cosmetic change before the kfree addition ?

Neil

Sergei Shtylyov

2015-10-06 11:15 UTC #5 lore raw

Hello.

Quoted 28 lines · Neil Armstrong

Quoted 28 lines
Quoted 20 lines
Quoted 20 lines
Quoted 12 lines
To prevent memory leakage on unbinding, add missing kfree calls.
Signed-off-by Neil Armstrong <[email protected]>
---
    net/dsa/dsa.c | 5 ++++-
    1 file changed, 4 insertions(+), 1 deletion(-)
net/dsa/dsa.c +0 -0
index c59fa5d..12cec40 100644
net/dsa/dsa.c static void dsa_remove_dst(struct dsa_switch_tree *dst) · line 917
Quoted 5 lines
net/dsa/dsa.c +1 -1
@@ -914,8 +914,10 @@ static void dsa_remove_dst(struct dsa_switch_tree *dst)
        for (i = 0; i < dst->pd->nr_chips; i++) {
            struct dsa_switch *ds = dst->ds[i];
 
-        if (ds != NULL)
+        if (ds != NULL) {
Didn;t scripts/checkpatch.pl complain here? just if (ds) is preferred in the networking code.
Quoted 5 lines
Quoted 2 lines
MBR, Sergei
Yes,
But I considered the cosmetic changes are not the subject of this serie.
Formally, all the patches should be checkpatch-clean...

Quoted 8 lines

Quoted 4 lines
Quoted 1 line
Neil
MBR, Sergei
Sure,
How should I handle this case ?
A separate patch with the cosmetic change before the kfree addition ?

No, that would classify as a cleanup, and cleanups shouldn't be queued
before fixes (should be the other way around). Just do this in this same patch, possibly mentioning in the change log.

Quoted 1 line

Neil

MBR, Sergei