All of lore.kernel.org
 help / color / mirror / Atom feed
From: gsantosh@codeaurora.org
To: cwchoi00@gmail.com, MyungJoo Ham <myungjoo.ham@samsung.com>,
	Kyungmin Park <kyungmin.park@samsung.com>,
	linux-pm@vger.kernel.org,
	linux-kernel <linux-kernel@vger.kernel.org>,
	"Rafael J. Wysocki" <rafael.j.wysocki@intel.com>
Cc: gsantosh@qti.qualcomm.com, skannan@quicinc.com
Subject: Re: [PATCH] devfreq: replace sscanf with kstrtol
Date: Mon, 07 Aug 2017 10:17:45 +0530	[thread overview]
Message-ID: <a5453d63a9c82cc1ffd4c2963a563200@codeaurora.org> (raw)
In-Reply-To: <CAGTfZH06PFTBu4VE_5AQjmpKfhr=TD9Qj8wcHpxQubCRtRMzzA@mail.gmail.com>

On 2017-08-04 20:42, Chanwoo Choi wrote:
> Hi,
> 
> On Fri, Aug 4, 2017 at 12:57 PM,  <gsantosh@codeaurora.org> 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@codeaurora.org>
>> 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@codeaurora.org>
>> ---
>>  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

  reply	other threads:[~2017-08-07  4:47 UTC|newest]

Thread overview: 6+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2017-08-04  3:57 [PATCH] devfreq: replace sscanf with kstrtol gsantosh
2017-08-04  4:23 ` gsantosh
2017-08-04 15:12 ` Chanwoo Choi
2017-08-07  4:47   ` gsantosh [this message]
2017-08-07  5:25     ` Chanwoo Choi
2017-08-07 12:26       ` gsantosh

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=a5453d63a9c82cc1ffd4c2963a563200@codeaurora.org \
    --to=gsantosh@codeaurora.org \
    --cc=cwchoi00@gmail.com \
    --cc=gsantosh@qti.qualcomm.com \
    --cc=kyungmin.park@samsung.com \
    --cc=linux-kernel@vger.kernel.org \
    --cc=linux-pm@vger.kernel.org \
    --cc=myungjoo.ham@samsung.com \
    --cc=rafael.j.wysocki@intel.com \
    --cc=skannan@quicinc.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.