Re: [PATCH] devfreq: replace sscanf with kstrtol

From: gsantosh
Date: Mon Aug 07 2017 - 00:47:52 EST


On 2017-08-04 20:42, Chanwoo Choi wrote:
Hi,

On Fri, Aug 4, 2017 at 12:57 PM, <gsantosh@xxxxxxxxxxxxxx> wrote:
Hi,

Adding error checks to devfreq userspace governor, the current
implementation results in setting wrong
frequency when sscanf returns error.


From 12e0a347addd70529b2c378299b27b65f0766f99 Mon Sep 17 00:00:00 2001
From: Santosh Mardi <gsantosh@xxxxxxxxxxxxxx>
Date: Tue, 25 Jul 2017 18:47:11 +0530
Subject: [PATCH] devfreq: replace sscanf with kstrtol

store_freq function of devfreq userspace governor
executes further, even if error is returned from sscanf,
this will result in setting up wrong frequency value.

The usage for the sscanf is only for single variable so
replace sscanf with kstrtol along with error check to
bail out if any error is returned.

Signed-off-by: Santosh Mardi <gsantosh@xxxxxxxxxxxxxx>
---
drivers/devfreq/governor_userspace.c | 5 ++++-
1 file changed, 4 insertions(+), 1 deletion(-)

diff --git a/drivers/devfreq/governor_userspace.c
b/drivers/devfreq/governor_userspace.c
index 77028c2..a84796d 100644
--- a/drivers/devfreq/governor_userspace.c
+++ b/drivers/devfreq/governor_userspace.c
@@ -53,12 +53,15 @@ static ssize_t store_freq(struct device *dev, struct
device_attribute *attr,
mutex_lock(&devfreq->lock);
data = devfreq->data;

- sscanf(buf, "%lu", &wanted);
+ err = kstrtol(buf, 0, &wanted);
+ if (err < 0)
+ goto out;

I think that just you can check the return value as following:
The other point of devfreq already uses the following style
to check the return value of sscanf. I think kstrtol is not necessary.

err = sscanf(buf, "%lu", &wanted);
if (err != 1)
goto out;


[Santosh] - I Agree we need to have this error check as mentioned by you if we are scanning an arrary from the sscanf,
but in the above code we are only scanning one variable and there is a rule in the checkpatch scripts, not to use sscanf if it is a single variable, So I need to replace sscanf to strtol

I have added all the mails I got as output from scripts/get_maintainer.pl scripts in this mail.


And please use the scripts/get_maintainer.pl
in order to prevent the missing of the reviewer.

data->user_frequency = wanted;
data->valid = true;
err = update_devfreq(devfreq);
if (err == 0)
err = count;
+out:
mutex_unlock(&devfreq->lock);
return err;
}
--

Regards,
Santosh M G.
Qualcomm Innovation Center