dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

[PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Alexander A. Klimov 1 week, 6 days ago

Depending on the user input, sscanf() may return 0 for 0 success.
But intr_coalesce_store() wants sscanf() to parse one number,
so expect 1 from sscanf(), not any int except -1.

While on it, fix typo in %du by using just %d,
as this interface expects %d or %d\n.
Latter made scripts/checkpatch.pl complain,
so use kstrtoint() instead of sscanf().

Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
---
 drivers/dma/ioat/sysfs.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
index e796ddb5383f..f59df569956a 100644
--- a/drivers/dma/ioat/sysfs.c
+++ b/drivers/dma/ioat/sysfs.c
@@ -144,7 +144,7 @@ size_t count)
 	int intr_coalesce = 0;
 	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
 
-	if (sscanf(page, "%du", &intr_coalesce) != -1) {
+	if (!kstrtoint(page, 10, &intr_coalesce)) {
 		if ((intr_coalesce < 0) ||
 		    (intr_coalesce > IOAT_INTRDELAY_MASK))
 			return -EINVAL;
-- 
2.54.0

Re: [PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Dave Jiang 1 week, 6 days ago


On 5/25/26 11:13 PM, Alexander A. Klimov wrote:
> Depending on the user input, sscanf() may return 0 for 0 success.
> But intr_coalesce_store() wants sscanf() to parse one number,
> so expect 1 from sscanf(), not any int except -1.
> 
> While on it, fix typo in %du by using just %d,
> as this interface expects %d or %d\n.
> Latter made scripts/checkpatch.pl complain,
> so use kstrtoint() instead of sscanf().
> 
> Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
> ---
>  drivers/dma/ioat/sysfs.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
> index e796ddb5383f..f59df569956a 100644
> --- a/drivers/dma/ioat/sysfs.c
> +++ b/drivers/dma/ioat/sysfs.c
> @@ -144,7 +144,7 @@ size_t count)
>  	int intr_coalesce = 0;
>  	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>  
> -	if (sscanf(page, "%du", &intr_coalesce) != -1) {
> +	if (!kstrtoint(page, 10, &intr_coalesce)) {

looks good. We can probably use kstrtouint() since we are expecting a positive number always.

DJ

>  		if ((intr_coalesce < 0) ||
>  		    (intr_coalesce > IOAT_INTRDELAY_MASK))
>  			return -EINVAL;

Re: [PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Alexander A. Klimov 1 week, 6 days ago


On 5/26/26 16:49, Dave Jiang wrote:
> 
> 
> On 5/25/26 11:13 PM, Alexander A. Klimov wrote:
>> Depending on the user input, sscanf() may return 0 for 0 success.
>> But intr_coalesce_store() wants sscanf() to parse one number,
>> so expect 1 from sscanf(), not any int except -1.
>>
>> While on it, fix typo in %du by using just %d,
>> as this interface expects %d or %d\n.
>> Latter made scripts/checkpatch.pl complain,
>> so use kstrtoint() instead of sscanf().
>>
>> Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>> ---
>>   drivers/dma/ioat/sysfs.c | 2 +-
>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
>> index e796ddb5383f..f59df569956a 100644
>> --- a/drivers/dma/ioat/sysfs.c
>> +++ b/drivers/dma/ioat/sysfs.c
>> @@ -144,7 +144,7 @@ size_t count)
>>   	int intr_coalesce = 0;
>>   	struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>>   
>> -	if (sscanf(page, "%du", &intr_coalesce) != -1) {
>> +	if (!kstrtoint(page, 10, &intr_coalesce)) {
> 
> looks good. We can probably use kstrtouint() since we are expecting a positive number always.

This would break `return -EINVAL;` below

> 
> DJ
> 
>>   		if ((intr_coalesce < 0) ||
>>   		    (intr_coalesce > IOAT_INTRDELAY_MASK))
>>   			return -EINVAL;
>

Re: [PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Dave Jiang 1 week, 4 days ago


On 5/26/26 11:06 AM, Alexander A. Klimov wrote:
> 
> 
> On 5/26/26 16:49, Dave Jiang wrote:
>>
>>
>> On 5/25/26 11:13 PM, Alexander A. Klimov wrote:
>>> Depending on the user input, sscanf() may return 0 for 0 success.
>>> But intr_coalesce_store() wants sscanf() to parse one number,
>>> so expect 1 from sscanf(), not any int except -1.
>>>
>>> While on it, fix typo in %du by using just %d,
>>> as this interface expects %d or %d\n.
>>> Latter made scripts/checkpatch.pl complain,
>>> so use kstrtoint() instead of sscanf().
>>>
>>> Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
>>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>>> ---
>>>   drivers/dma/ioat/sysfs.c | 2 +-
>>>   1 file changed, 1 insertion(+), 1 deletion(-)
>>>
>>> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
>>> index e796ddb5383f..f59df569956a 100644
>>> --- a/drivers/dma/ioat/sysfs.c
>>> +++ b/drivers/dma/ioat/sysfs.c
>>> @@ -144,7 +144,7 @@ size_t count)
>>>       int intr_coalesce = 0;
>>>       struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>>>   -    if (sscanf(page, "%du", &intr_coalesce) != -1) {
>>> +    if (!kstrtoint(page, 10, &intr_coalesce)) {
>>
>> looks good. We can probably use kstrtouint() since we are expecting a positive number always.
> 
> This would break `return -EINVAL;` below

Shouldn't we just drop the < 0 compare since it's no longer needed?

> 
>>
>> DJ
>>
>>>           if ((intr_coalesce < 0) ||
>>>               (intr_coalesce > IOAT_INTRDELAY_MASK))
>>>               return -EINVAL;
>>
>

Re: [PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Alexander A. Klimov 1 week, 1 day ago


On 5/28/26 22:06, Dave Jiang wrote:
> 
> 
> On 5/26/26 11:06 AM, Alexander A. Klimov wrote:
>>
>>
>> On 5/26/26 16:49, Dave Jiang wrote:
>>>
>>>
>>> On 5/25/26 11:13 PM, Alexander A. Klimov wrote:
>>>> Depending on the user input, sscanf() may return 0 for 0 success.
>>>> But intr_coalesce_store() wants sscanf() to parse one number,
>>>> so expect 1 from sscanf(), not any int except -1.
>>>>
>>>> While on it, fix typo in %du by using just %d,
>>>> as this interface expects %d or %d\n.
>>>> Latter made scripts/checkpatch.pl complain,
>>>> so use kstrtoint() instead of sscanf().
>>>>
>>>> Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
>>>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>>>> ---
>>>>    drivers/dma/ioat/sysfs.c | 2 +-
>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>
>>>> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
>>>> index e796ddb5383f..f59df569956a 100644
>>>> --- a/drivers/dma/ioat/sysfs.c
>>>> +++ b/drivers/dma/ioat/sysfs.c
>>>> @@ -144,7 +144,7 @@ size_t count)
>>>>        int intr_coalesce = 0;
>>>>        struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>>>>    -    if (sscanf(page, "%du", &intr_coalesce) != -1) {
>>>> +    if (!kstrtoint(page, 10, &intr_coalesce)) {
>>>
>>> looks good. We can probably use kstrtouint() since we are expecting a positive number always.
>>
>> This would break `return -EINVAL;` below
> 
> Shouldn't we just drop the < 0 compare since it's no longer needed?

Wouldn't that change behavior shown to userspace from return -EINVAL
on negative int input to return count?

Re: [PATCH] dmaengine: ioatdma: use !kstrtoint(), not sscanf()!=-1

Posted by Dave Jiang 1 week ago


On 5/31/26 1:56 AM, Alexander A. Klimov wrote:
> 
> 
> On 5/28/26 22:06, Dave Jiang wrote:
>>
>>
>> On 5/26/26 11:06 AM, Alexander A. Klimov wrote:
>>>
>>>
>>> On 5/26/26 16:49, Dave Jiang wrote:
>>>>
>>>>
>>>> On 5/25/26 11:13 PM, Alexander A. Klimov wrote:
>>>>> Depending on the user input, sscanf() may return 0 for 0 success.
>>>>> But intr_coalesce_store() wants sscanf() to parse one number,
>>>>> so expect 1 from sscanf(), not any int except -1.
>>>>>
>>>>> While on it, fix typo in %du by using just %d,
>>>>> as this interface expects %d or %d\n.
>>>>> Latter made scripts/checkpatch.pl complain,
>>>>> so use kstrtoint() instead of sscanf().
>>>>>
>>>>> Fixes: 268e2519f5b7 ("dmaengine: ioatdma: Add intr_coalesce sysfs entry")
>>>>> Signed-off-by: Alexander A. Klimov <grandmaster@al2klimov.de>
>>>>> ---
>>>>>    drivers/dma/ioat/sysfs.c | 2 +-
>>>>>    1 file changed, 1 insertion(+), 1 deletion(-)
>>>>>
>>>>> diff --git a/drivers/dma/ioat/sysfs.c b/drivers/dma/ioat/sysfs.c
>>>>> index e796ddb5383f..f59df569956a 100644
>>>>> --- a/drivers/dma/ioat/sysfs.c
>>>>> +++ b/drivers/dma/ioat/sysfs.c
>>>>> @@ -144,7 +144,7 @@ size_t count)
>>>>>        int intr_coalesce = 0;
>>>>>        struct ioatdma_chan *ioat_chan = to_ioat_chan(c);
>>>>>    -    if (sscanf(page, "%du", &intr_coalesce) != -1) {
>>>>> +    if (!kstrtoint(page, 10, &intr_coalesce)) {
>>>>
>>>> looks good. We can probably use kstrtouint() since we are expecting a positive number always.
>>>
>>> This would break `return -EINVAL;` below
>>
>> Shouldn't we just drop the < 0 compare since it's no longer needed?
> 
> Wouldn't that change behavior shown to userspace from return -EINVAL
> on negative int input to return count?

No a negative value would trigger parsing error and return -EINVAL. Same behavior for user.