Re: [PATCH] r8152: simplify loops in generic_ocp_{read,write}()

On 8/23/26 11:11 AM, David Laight wrote:
[...]

>>> In generic_ocp_{read,write}(), the *while* loops look very strange:
>>> the last iteration is executed differently to the prior ones, doing
>>> some useless assignments before *break*. Move the code for the last
>>> iteration out of the loop bodies, dropping the pointless statements
>>> as well...
>>>
>>> Found by Linux Verification Center (linuxtesting.org) with the Svace
>>> static analysis tool.
>>>
>>> Signed-off-by: Sergey Shtylyov <[email protected]>
>>
>>    Actually, scratch this patch -- it's not entirely correct... :-/
>>
>> [...]
>>
>>> diff --git a/drivers/net/usb/r8152.c b/drivers/net/usb/r8152.c>>> index f61686433031..de9738bdce85 100644
>>> --- a/drivers/net/usb/r8152.c
>>> +++ b/drivers/net/usb/r8152.c
>>> @@ -1431,27 +1431,19 @@ static int generic_ocp_read(struct r8152 *tp, u16 index, u16 size,
>>>     if ((u32)index + (u32)size > 0xffff)
>>>             return -EPERM;
>>>
>>> -   while (size) {
>>> -           if (size > limit) {
>>> -                   ret = get_registers(tp, index, type, limit, data);
>>> -                   if (ret < 0)
>>> -                           break;
>>> -
>>> -                   index += limit;
>>> -                   data += limit;
>>> -                   size -= limit;
>>> -           } else {
>>> -                   ret = get_registers(tp, index, type, size, data);
>>> -                   if (ret < 0)
>>> -                           break;
>>> +   while (size > limit) {
>>> +           ret = get_registers(tp, index, type, limit, data);
>>> +           if (ret < 0)
>>> +                   goto error1;
>>>
>>> -                   index += size;
>>> -                   data += size;
>>> -                   size = 0;
>>> -                   break;
>>> -           }
>>> +           index += limit;
>>> +           data += limit;
>>> +           size -= limit;
>>>     }
>>>
>>
>>    I forgot to check size for 0 here...
> 
> I don't think it can be zero - assuming it isn't zero on entry.

   Even if so, anyways it can -- if size % limit == 0 on entry...

> David
[...]

MBR, Sergey