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

[David Laight <david.laight.linux@gmail.com>]

On Sun, 23 Aug 2026 11:20:30 +0300
Sergey Shtylyov <s.shtylyov@auroraos.dev> wrote:

> 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 <s.shtylyov@auroraos.dev>  
> >>
> >>    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...

Not with the 'size > limit' check at the top of the loop.

David

> 
> > David  
> [...]
> 
> MBR, Sergey
> 
>