Skip to contentSkip to conversation

PM / devfreq: fix indentation in devfreq_add_device()

PATCH In review Yangtao Li 4 participants 8 messages Jan 19, 2019 – Jan 21, 2019 Device frequency (devfreq)

lore
Share & tools
lore

Yangtao Li

author
#0 lore raw

PATCH 1/3 PM / devfreq: fix indentation in devfreq_add_device()

To beautify the code format.

Signed-off-by Yangtao Li <[email protected]>
---
 drivers/devfreq/devfreq.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/devfreq/devfreq.c +8 -9
index 0ae3de76833b..076b1c2f40a4 100644
@@ -683,16 +683,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_out;
 	}
 
-	devfreq->trans_table =
-		devm_kzalloc(&devfreq->dev,
-			     array3_size(sizeof(unsigned int),
-					 devfreq->profile->max_state,
-					 devfreq->profile->max_state),
-			     GFP_KERNEL);
+	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
+					    array3_size(sizeof(unsigned int),
+					    devfreq->profile->max_state,
+					    devfreq->profile->max_state),
+					    GFP_KERNEL);
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
-						devfreq->profile->max_state,
-						sizeof(unsigned long),
-						GFP_KERNEL);
+					      devfreq->profile->max_state,
+					      sizeof(unsigned long),
+					      GFP_KERNEL);
 	devfreq->last_stat_updated = jiffies;
 
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);
Signature · 1 line
2.17.0
author
#1 lore raw

PATCH 2/3 PM / devfreq: fix missing check of return value in devfreq_add_device()

devm_kzalloc() could fail, so insert a check of its return value. And if it fails, returns -ENOMEM.

Signed-off-by Yangtao Li <[email protected]>
---
 drivers/devfreq/devfreq.c | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)
drivers/devfreq/devfreq.c +13 -1
index 076b1c2f40a4..923889229a0b 100644
@@ -688,10 +688,22 @@ struct devfreq *devfreq_add_device(struct device *dev,
 					    devfreq->profile->max_state,
 					    devfreq->profile->max_state),
 					    GFP_KERNEL);
+	if (!devfreq->trans_table) {
+		mutex_unlock(&devfreq->lock);
+		err = -ENOMEM;
+		goto err_devfreq;
+	}
+
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
 					      devfreq->profile->max_state,
 					      sizeof(unsigned long),
 					      GFP_KERNEL);
+	if (!devfreq->time_in_state) {
+		mutex_unlock(&devfreq->lock);
+		err = -ENOMEM;
+		goto err_devfreq;
+	}
+
 	devfreq->last_stat_updated = jiffies;
 
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);

@@ -725,7 +737,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 
 err_init:
 	mutex_unlock(&devfreq_list_lock);
-
+err_devfreq:
 	devfreq_remove_device(devfreq);
 	devfreq = NULL;
 err_dev:
Signature · 1 line
2.17.0
maintainer Acked-by on v1 2/3
#2 lore raw

RE: [PATCH 2/3] PM / devfreq: fix missing check of return value in devfreq_add_device()

drivers/devfreq/devfreq.c

Quoted 12 lines

devm_kzalloc() could fail, so insert a check of its return value. And if it fails, returns -ENOMEM.

Signed-off-by Yangtao Li <[email protected]>

---
drivers/devfreq/devfreq.c | 14 +++++++++++++-

1 file changed, 13 insertions(+), 1 deletion(-)
drivers/devfreq/devfreq.c +0 -0
index 076b1c2f40a4..923889229a0b 100644
Acked-by MyungJoo Ham <[email protected]>
author
#3 lore raw

PATCH 3/3 PM / devfreq: fix mem leak in devfreq_add_device()

In reply to Yangtao Li

'devfreq' is malloced in devfreq_add_device() and should be freed in the error handling cases, otherwise it will cause memory leak.

Signed-off-by Yangtao Li <[email protected]>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/devfreq/devfreq.c +1 -1
index 923889229a0b..fe6c157cb7e0 100644
@@ -651,7 +651,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		mutex_unlock(&devfreq->lock);
 		err = set_freq_table(devfreq);
 		if (err < 0)
-			goto err_out;
+			goto err_dev;
 		mutex_lock(&devfreq->lock);
 	}
Signature · 1 line
2.17.0
maintainer Acked-by on v1 3/3
#4 lore raw

RE: [PATCH 3/3] PM / devfreq: fix mem leak in devfreq_add_device()

Quoted 4 lines

'devfreq' is malloced in devfreq_add_device() and should be freed in the error handling cases, otherwise it will cause memory leak.

Signed-off-by Yangtao Li <[email protected]>
Acked-by MyungJoo Ham <[email protected]>
Quoted 17 lines

---
drivers/devfreq/devfreq.c | 2 +-

1 file changed, 1 insertion(+), 1 deletion(-)
drivers/devfreq/devfreq.c +1 -1
index 923889229a0b..fe6c157cb7e0 100644
@@ -651,7 +651,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		mutex_unlock(&devfreq->lock);
 		err = set_freq_table(devfreq);
 		if (err < 0)
-			goto err_out;
+			goto err_dev;
 		mutex_lock(&devfreq->lock);
 	}
maintainer Reviewed-by on v1 3/3
#5 lore raw

Re: [PATCH 3/3] PM / devfreq: fix mem leak in devfreq_add_device()

In reply to Yangtao Li

Hi,

Quoted 14 lines · Yangtao Li

'devfreq' is malloced in devfreq_add_device() and should be freed in the error handling cases, otherwise it will cause memory leak.

Signed-off-by Yangtao Li <[email protected]>
---
 drivers/devfreq/devfreq.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
drivers/devfreq/devfreq.c +0 -0
index 923889229a0b..fe6c157cb7e0 100644

drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 657

Quoted 8 lines
drivers/devfreq/devfreq.c +1 -1
@@ -651,7 +651,7 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		mutex_unlock(&devfreq->lock);
 		err = set_freq_table(devfreq);
 		if (err < 0)
-			goto err_out;
+			goto err_dev;
 		mutex_lock(&devfreq->lock);
 	}
Reviewed-by Chanwoo Choi <[email protected]>
Signature · 3 lines
Best Regards,
Chanwoo Choi
Samsung Electronics
maintainer Reviewed-by on v1 1/3
#6 lore raw
In reply to Yangtao Li

Hi,

Quoted 27 lines · Yangtao Li

To beautify the code format.

Signed-off-by Yangtao Li <[email protected]>
---
 drivers/devfreq/devfreq.c | 17 ++++++++---------
 1 file changed, 8 insertions(+), 9 deletions(-)
drivers/devfreq/devfreq.c +5 -6
index 0ae3de76833b..076b1c2f40a4 100644
@@ -683,16 +683,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_out;
 	}
 
-	devfreq->trans_table =
-		devm_kzalloc(&devfreq->dev,
-			     array3_size(sizeof(unsigned int),
-					 devfreq->profile->max_state,
-					 devfreq->profile->max_state),
-			     GFP_KERNEL);
+	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
+					    array3_size(sizeof(unsigned int),
+					    devfreq->profile->max_state,
+					    devfreq->profile->max_state),
+					    GFP_KERNEL);

drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 697

Quoted 10 lines
drivers/devfreq/devfreq.c +3 -3
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
-						devfreq->profile->max_state,
-						sizeof(unsigned long),
-						GFP_KERNEL);
+					      devfreq->profile->max_state,
+					      sizeof(unsigned long),
+					      GFP_KERNEL);
 	devfreq->last_stat_updated = jiffies;
 
 	srcu_init_notifier_head(&devfreq->transition_notifier_list);
Reviewed-by Chanwoo Choi <[email protected]>
Signature · 3 lines
Best Regards,
Chanwoo Choi
Samsung Electronics
#7 lore raw
Quoted 2 lines · Chanwoo Choi
Quoted 1 line · Yangtao Li

To beautify the code format.

I believe half of the changes are actually _less_ beautiful.

Quoted 1 line

drivers/devfreq/devfreq.c

Quoted 1 line
drivers/devfreq/devfreq.c +0 -0

[]

Quoted 4 lines
Quoted 4 lines
drivers/devfreq/devfreq.c +0 -0
@@ -683,16 +683,15 @@ struct devfreq *devfreq_add_device(struct device *dev,
 		goto err_out;
 	}
Quoted 11 lines

drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 690

Quoted 11 lines
drivers/devfreq/devfreq.c +5 -6
-	devfreq->trans_table =
-		devm_kzalloc(&devfreq->dev,
-			     array3_size(sizeof(unsigned int),
-					 devfreq->profile->max_state,
-					 devfreq->profile->max_state),
-			     GFP_KERNEL);
+	devfreq->trans_table = devm_kzalloc(&devfreq->dev,
+					    array3_size(sizeof(unsigned int),
+					    devfreq->profile->max_state,
+					    devfreq->profile->max_state),
+					    GFP_KERNEL);

I think this bit is worse because the array3_size arguments are no longer aligned.

Quoted 7 lines

drivers/devfreq/devfreq.c struct devfreq *devfreq_add_device(struct device *dev, · line 694

Quoted 7 lines
drivers/devfreq/devfreq.c +3 -3
 	devfreq->time_in_state = devm_kcalloc(&devfreq->dev,
-						devfreq->profile->max_state,
-						sizeof(unsigned long),
-						GFP_KERNEL);
+					      devfreq->profile->max_state,
+					      sizeof(unsigned long),
+					      GFP_KERNEL);

and this bit is better.