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

[Dave Jiang]

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@xxxxxxxxxxxx>
>>>>> ---
>>>>>    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.