[PATCH 1/2] iio: gyro: check sscanf return value

[PATCH 1/2] iio: gyro: check sscanf return value

[Ioana Ciornei]

This patch fixes the checkpatch warnings:
WARNING: unchecked sscanf return value

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---

based on linux-iio/testing branch

 drivers/iio/gyro/adis16136.c | 4 +++-
 1 file changed, 3 insertions(+), 1 deletion(-)

diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
index 26de876..bb09bff 100644
--- a/drivers/iio/gyro/adis16136.c
+++ b/drivers/iio/gyro/adis16136.c
@@ -435,7 +435,9 @@ static int adis16136_initial_setup(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	sscanf(indio_dev->name, "adis%u\n", &device_id);
+	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
+	if (ret != 1)
+		return -EINVAL;
 
 	if (prod_id != device_id)
 		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
-- 
2.1.4

[PATCH 2/2] iio: imu: check sscanf return value

[Ioana Ciornei]

This patch fixes the following checkpatch warning:
WARNING: unchecked sscanf return value

Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
---

based on linux-iio testing branch

 drivers/iio/imu/adis16400_core.c | 6 +++++-
 drivers/iio/imu/adis16480.c      | 4 +++-
 2 files changed, 8 insertions(+), 2 deletions(-)

diff --git a/drivers/iio/imu/adis16400_core.c b/drivers/iio/imu/adis16400_core.c
index abc4c50..72bcc24 100644
--- a/drivers/iio/imu/adis16400_core.c
+++ b/drivers/iio/imu/adis16400_core.c
@@ -288,7 +288,11 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
 		if (ret)
 			goto err_ret;
 
-		sscanf(indio_dev->name, "adis%u\n", &device_id);
+		ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
+		if (ret != 1) {
+			ret = -EINVAL;
+			goto err_ret;
+		}
 
 		if (prod_id != device_id)
 			dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
index b94bfd3..16d4304 100644
--- a/drivers/iio/imu/adis16480.c
+++ b/drivers/iio/imu/adis16480.c
@@ -765,7 +765,9 @@ static int adis16480_initial_setup(struct iio_dev *indio_dev)
 	if (ret)
 		return ret;
 
-	sscanf(indio_dev->name, "adis%u\n", &device_id);
+	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
+	if (ret != 1)
+		return -EINVAL;
 
 	if (prod_id != device_id)
 		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
-- 
2.1.4

Re: [PATCH 1/2] iio: gyro: check sscanf return value

[Jonathan Cameron]

On 01/11/15 12:58, Ioana Ciornei wrote:
> This patch fixes the checkpatch warnings:
> WARNING: unchecked sscanf return value
> 
> Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
Hi Ioana,

Couple of minor process points (the patches are fine!)

You should have cc'd the driver author and generally also
the various listed reviewers in MAINTAINERS.  In this case
of of them is the driver author so possibly just pinging him
is the way to go.  In the case of Analog devices parts,
Lars-Peter is also the maintainer so it's all about as
easy as it gets in this case.

Lars, looks fine to me.  Your driver so please sanity check.
Clearly it's not a bug as the inputs are entirely controlled
by the driver code, but I think having a sanity check on
the return value is a good thing (from the point of view of
best practice). 

There is a second patch for some the IIO drivers that are your
problem as well - I'll let you pull that off the list as I'm
feeling lazy ;)

I did have to check that sscanf never returns any more helpful
error codes, but unlike the glibc one it seems not.  What fun.

Jonathan
> ---
> 
> based on linux-iio/testing branch
> 
>  drivers/iio/gyro/adis16136.c | 4 +++-
>  1 file changed, 3 insertions(+), 1 deletion(-)
> 
> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
> index 26de876..bb09bff 100644
> --- a/drivers/iio/gyro/adis16136.c
> +++ b/drivers/iio/gyro/adis16136.c
> @@ -435,7 +435,9 @@ static int adis16136_initial_setup(struct iio_dev *indio_dev)
>  	if (ret)
>  		return ret;
>  
> -	sscanf(indio_dev->name, "adis%u\n", &device_id);
> +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> +	if (ret != 1)
> +		return -EINVAL;
>  
>  	if (prod_id != device_id)
>  		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> 

Re: [PATCH 1/2] iio: gyro: check sscanf return value

[Ioana Ciornei]

On Sun, Nov 1, 2015 at 8:17 PM, Jonathan Cameron <jic23@kernel.org> wrote:
> On 01/11/15 12:58, Ioana Ciornei wrote:
>> This patch fixes the checkpatch warnings:
>> WARNING: unchecked sscanf return value
>>
>> Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
> Hi Ioana,
>
> Couple of minor process points (the patches are fine!)
>
> You should have cc'd the driver author and generally also
> the various listed reviewers in MAINTAINERS.  In this case
> of of them is the driver author so possibly just pinging him
> is the way to go.  In the case of Analog devices parts,
> Lars-Peter is also the maintainer so it's all about as
> easy as it gets in this case.

Ok. From now on I will cc the maintainers and the reviewers.
Sorry for the trouble.

Ioana

>
> Lars, looks fine to me.  Your driver so please sanity check.
> Clearly it's not a bug as the inputs are entirely controlled
> by the driver code, but I think having a sanity check on
> the return value is a good thing (from the point of view of
> best practice).
>
> There is a second patch for some the IIO drivers that are your
> problem as well - I'll let you pull that off the list as I'm
> feeling lazy ;)
>
> I did have to check that sscanf never returns any more helpful
> error codes, but unlike the glibc one it seems not.  What fun.
>
> Jonathan
>> ---
>>
>> based on linux-iio/testing branch
>>
>>  drivers/iio/gyro/adis16136.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
>> index 26de876..bb09bff 100644
>> --- a/drivers/iio/gyro/adis16136.c
>> +++ b/drivers/iio/gyro/adis16136.c
>> @@ -435,7 +435,9 @@ static int adis16136_initial_setup(struct iio_dev *indio_dev)
>>       if (ret)
>>               return ret;
>>
>> -     sscanf(indio_dev->name, "adis%u\n", &device_id);
>> +     ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
>> +     if (ret != 1)
>> +             return -EINVAL;
>>
>>       if (prod_id != device_id)
>>               dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
>>
>



-- 
Ioana Ciornei
Facultatea de Automatica si Calculatoare, UPB
Tel. 0753 861 668

Re: [PATCH 1/2] iio: gyro: check sscanf return value

[Lars-Peter Clausen]

On 11/01/2015 07:17 PM, Jonathan Cameron wrote:
> On 01/11/15 12:58, Ioana Ciornei wrote:
>> This patch fixes the checkpatch warnings:
>> WARNING: unchecked sscanf return value
>>
>> Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
> Hi Ioana,
> 
> Couple of minor process points (the patches are fine!)
> 
> You should have cc'd the driver author and generally also
> the various listed reviewers in MAINTAINERS.  In this case
> of of them is the driver author so possibly just pinging him
> is the way to go.  In the case of Analog devices parts,
> Lars-Peter is also the maintainer so it's all about as
> easy as it gets in this case.
> 
> Lars, looks fine to me.  Your driver so please sanity check.
> Clearly it's not a bug as the inputs are entirely controlled
> by the driver code, but I think having a sanity check on
> the return value is a good thing (from the point of view of
> best practice). 

As you say strictly speaking it is not necessary, but if it makes checkpatch
happy why not.

Acked-by: Lars-Peter Clausen <lars@metafoo.de>


> 
> There is a second patch for some the IIO drivers that are your
> problem as well - I'll let you pull that off the list as I'm
> feeling lazy ;)
> 
> I did have to check that sscanf never returns any more helpful
> error codes, but unlike the glibc one it seems not.  What fun.
> 
> Jonathan
>> ---
>>
>> based on linux-iio/testing branch
>>
>>  drivers/iio/gyro/adis16136.c | 4 +++-
>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>
>> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
>> index 26de876..bb09bff 100644
>> --- a/drivers/iio/gyro/adis16136.c
>> +++ b/drivers/iio/gyro/adis16136.c
>> @@ -435,7 +435,9 @@ static int adis16136_initial_setup(struct iio_dev *indio_dev)
>>  	if (ret)
>>  		return ret;
>>  
>> -	sscanf(indio_dev->name, "adis%u\n", &device_id);
>> +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
>> +	if (ret != 1)
>> +		return -EINVAL;
>>  
>>  	if (prod_id != device_id)
>>  		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 1/2] iio: gyro: check sscanf return value

[Jonathan Cameron]

On 03/11/15 09:22, Lars-Peter Clausen wrote:
> On 11/01/2015 07:17 PM, Jonathan Cameron wrote:
>> On 01/11/15 12:58, Ioana Ciornei wrote:
>>> This patch fixes the checkpatch warnings:
>>> WARNING: unchecked sscanf return value
>>>
>>> Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
>> Hi Ioana,
>>
>> Couple of minor process points (the patches are fine!)
>>
>> You should have cc'd the driver author and generally also
>> the various listed reviewers in MAINTAINERS.  In this case
>> of of them is the driver author so possibly just pinging him
>> is the way to go.  In the case of Analog devices parts,
>> Lars-Peter is also the maintainer so it's all about as
>> easy as it gets in this case.
>>
>> Lars, looks fine to me.  Your driver so please sanity check.
>> Clearly it's not a bug as the inputs are entirely controlled
>> by the driver code, but I think having a sanity check on
>> the return value is a good thing (from the point of view of
>> best practice). 
> 
> As you say strictly speaking it is not necessary, but if it makes checkpatch
> happy why not.
> 
> Acked-by: Lars-Peter Clausen <lars@metafoo.de>
Applied to the togreg branch of iio.git - initially pushed out as testing
for the autobuilders to try and break it.

Thanks,

jonathan
> 
> 
>>
>> There is a second patch for some the IIO drivers that are your
>> problem as well - I'll let you pull that off the list as I'm
>> feeling lazy ;)
>>
>> I did have to check that sscanf never returns any more helpful
>> error codes, but unlike the glibc one it seems not.  What fun.
>>
>> Jonathan
>>> ---
>>>
>>> based on linux-iio/testing branch
>>>
>>>  drivers/iio/gyro/adis16136.c | 4 +++-
>>>  1 file changed, 3 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/iio/gyro/adis16136.c b/drivers/iio/gyro/adis16136.c
>>> index 26de876..bb09bff 100644
>>> --- a/drivers/iio/gyro/adis16136.c
>>> +++ b/drivers/iio/gyro/adis16136.c
>>> @@ -435,7 +435,9 @@ static int adis16136_initial_setup(struct iio_dev *indio_dev)
>>>  	if (ret)
>>>  		return ret;
>>>  
>>> -	sscanf(indio_dev->name, "adis%u\n", &device_id);
>>> +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
>>> +	if (ret != 1)
>>> +		return -EINVAL;
>>>  
>>>  	if (prod_id != device_id)
>>>  		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
>>>
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>>
> 
> --
> To unsubscribe from this list: send the line "unsubscribe linux-iio" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 

Re: [PATCH 2/2] iio: imu: check sscanf return value

[Jonathan Cameron]

On 01/11/15 12:58, Ioana Ciornei wrote:
> This patch fixes the following checkpatch warning:
> WARNING: unchecked sscanf return value
> 
> Signed-off-by: Ioana Ciornei <ciorneiioana@gmail.com>
Applied to the togreg branch of stating - initially pushed
out as testing for the autobuilders to play with it.

Thanks,

Jonathan
> ---
> 
> based on linux-iio testing branch
> 
>  drivers/iio/imu/adis16400_core.c | 6 +++++-
>  drivers/iio/imu/adis16480.c      | 4 +++-
>  2 files changed, 8 insertions(+), 2 deletions(-)
> 
> diff --git a/drivers/iio/imu/adis16400_core.c b/drivers/iio/imu/adis16400_core.c
> index abc4c50..72bcc24 100644
> --- a/drivers/iio/imu/adis16400_core.c
> +++ b/drivers/iio/imu/adis16400_core.c
> @@ -288,7 +288,11 @@ static int adis16400_initial_setup(struct iio_dev *indio_dev)
>  		if (ret)
>  			goto err_ret;
>  
> -		sscanf(indio_dev->name, "adis%u\n", &device_id);
> +		ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> +		if (ret != 1) {
> +			ret = -EINVAL;
> +			goto err_ret;
> +		}
>  
>  		if (prod_id != device_id)
>  			dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
> diff --git a/drivers/iio/imu/adis16480.c b/drivers/iio/imu/adis16480.c
> index b94bfd3..16d4304 100644
> --- a/drivers/iio/imu/adis16480.c
> +++ b/drivers/iio/imu/adis16480.c
> @@ -765,7 +765,9 @@ static int adis16480_initial_setup(struct iio_dev *indio_dev)
>  	if (ret)
>  		return ret;
>  
> -	sscanf(indio_dev->name, "adis%u\n", &device_id);
> +	ret = sscanf(indio_dev->name, "adis%u\n", &device_id);
> +	if (ret != 1)
> +		return -EINVAL;
>  
>  	if (prod_id != device_id)
>  		dev_warn(&indio_dev->dev, "Device ID(%u) and product ID(%u) do not match.",
>