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

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

[Sergey Shtylyov]

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>

---
 drivers/net/usb/r8152.c | 67 ++++++++++++++++-------------------------
 1 file changed, 26 insertions(+), 41 deletions(-)

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;
 	}
 
+	ret = get_registers(tp, index, type, size, data);
+
+error1:
 	if (ret == -ENODEV)
 		rtl_set_unplug(tp);
 
@@ -1498,31 +1490,24 @@ static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
 		if (byen != BYTE_EN_DWORD)
 			size -= 4;
 
-		while (size) {
-			if (size > limit) {
-				ret = set_registers(tp, index,
-						    type | BYTE_EN_DWORD,
-						    limit, data);
-				if (ret < 0)
-					goto error1;
-
-				index += limit;
-				data += limit;
-				size -= limit;
-			} else {
-				ret = set_registers(tp, index,
-						    type | BYTE_EN_DWORD,
-						    size, data);
-				if (ret < 0)
-					goto error1;
-
-				index += size;
-				data += size;
-				size = 0;
-				break;
-			}
+		while (size > limit) {
+			ret = set_registers(tp, index, type | BYTE_EN_DWORD,
+					    limit, data);
+			if (ret < 0)
+				goto error1;
+
+			index += limit;
+			data += limit;
+			size -= limit;
 		}
 
+		ret = set_registers(tp, index, type | BYTE_EN_DWORD, size, data);
+		if (ret < 0)
+			goto error1;
+
+		index += size;
+		data += size;
+
 		/* Set the last DWORD */
 		if (byen != BYTE_EN_DWORD)
 			ret = set_registers(tp, index, type | byen, 4, data);
-- 
2.55.0

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

[Sergey Shtylyov]

On 8/22/26 11:21 PM, Sergey Shtylyov 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>
[...]

   And of course I forgot [net-next] in the subject... :-)

MBR, Sergey

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

[Jakub Kicinski]

On Sat, 22 Aug 2026 23:26:30 +0300 Sergey Shtylyov wrote:
>    And of course I forgot [net-next] in the subject... :-)

And to check if it's open ;)
https://netdev.bots.linux.dev/net-next.html
-- 
pw-bot: defer

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

[Sergey Shtylyov]

On 8/22/26 11:58 PM, Jakub Kicinski wrote:
[...]

>>    And of course I forgot [net-next] in the subject... :-)
> 
> And to check if it's open ;)
> https://netdev.bots.linux.dev/net-next.html

   I don't do much netdev these days, so I just forgot the usual
routine (and I couldn't wait so much with posting the patch anyway).
Is it still possible to send net-next stuff with [RFC] added to
the subject while net-next is closed?
   As I'll have to do v2 anyway, I'll try to wait for net-next to
open before posting it...

[...]

MBR, Sergey

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

[Jakub Kicinski]

On Sun, 23 Aug 2026 11:37:11 +0300 Sergey Shtylyov wrote:
> > And to check if it's open ;)
> > https://netdev.bots.linux.dev/net-next.html  
> 
>    I don't do much netdev these days, so I just forgot the usual
> routine (and I couldn't wait so much with posting the patch anyway).
> Is it still possible to send net-next stuff with [RFC] added to
> the subject while net-next is closed?

Yup, RFC is fine (tho FWIW CI coverage of RFCs is limited for cost
reasons)

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

[Michal Pecio]

On Sat, 22 Aug 2026 23:21:54 +0300, Sergey Shtylyov 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>
> 
> ---
>  drivers/net/usb/r8152.c | 67 ++++++++++++++++-------------------------
>  1 file changed, 26 insertions(+), 41 deletions(-)
> 
> 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;
>  	}
>  
> +	ret = get_registers(tp, index, type, size, data);
> +
> +error1:
>  	if (ret == -ENODEV)
>  		rtl_set_unplug(tp);

Looks like it could be shorter still.

s/limit/chunk/

while (size) {
	if (size < chunk)
		chunk = size;
	ret = get_registers(tp, index, type, chunk, data);
	if (ret < 0)
		break;
	index += chunk;
	data += chunk;
	size -= chunk;
}

Then it could be do-while, because we know size > 0, though
I suppose compilers may figure it out themselves anyway.

Regards,
Michal

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

[David Laight]

On Sat, 22 Aug 2026 23:03:26 +0200
Michal Pecio <michal.pecio@gmail.com> wrote:

> On Sat, 22 Aug 2026 23:21:54 +0300, Sergey Shtylyov 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>
> > 
> > ---
> >  drivers/net/usb/r8152.c | 67 ++++++++++++++++-------------------------
> >  1 file changed, 26 insertions(+), 41 deletions(-)
> > 
> > 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;
> >  	}
> >  
> > +	ret = get_registers(tp, index, type, size, data);
> > +
> > +error1:
> >  	if (ret == -ENODEV)
> >  		rtl_set_unplug(tp);  
> 
> Looks like it could be shorter still.
> 
> s/limit/chunk/
> 
> while (size) {
> 	if (size < chunk)
> 		chunk = size;

I think you meant:
	chunk = min(size, limit);

> 	ret = get_registers(tp, index, type, chunk, data);
> 	if (ret < 0)
> 		break;
> 	index += chunk;
> 	data += chunk;
> 	size -= chunk;
> }

That is the usual pattern...
Although may you need to be careful to stop min() bleating if size
is a signed type.
Changing the loop to 'while (size > 0)' can be enough.

David

> 
> Then it could be do-while, because we know size > 0, though
> I suppose compilers may figure it out themselves anyway.
> 
> Regards,
> Michal
> 

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

[Sergey Shtylyov]

On 8/23/26 12:03 AM, Michal Pecio 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>
>>
>> ---
>>  drivers/net/usb/r8152.c | 67 ++++++++++++++++-------------------------
>>  1 file changed, 26 insertions(+), 41 deletions(-)
>>
>> 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;
>>       }
>>
>> +     ret = get_registers(tp, index, type, size, data);
>> +
>> +error1:
>>       if (ret == -ENODEV)
>>               rtl_set_unplug(tp);
> 
> Looks like it could be shorter still.
> 
> s/limit/chunk/
> 
> while (size) {
>         if (size < chunk)
>                 chunk = size;
>         ret = get_registers(tp, index, type, chunk, data);
>         if (ret < 0)
>                 break;
>         index += chunk;
>         data += chunk;
>         size -= chunk;
> }

   That's definitely better, thank you! :-)
   I'll rewrite the patch along these lines, mentioning you in the
Suggested-by tag...

> Then it could be do-while, because we know size > 0, though
> I suppose compilers may figure it out themselves anyway.

   I'd prefer to leave handling of !size as it is now, so I'll
keep using the *while* loop...

> Regards,
> Michal

MBR, Sergey

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

[Sergey Shtylyov]

On 8/22/26 11:21 PM, Sergey Shtylyov 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...

> +	ret = get_registers(tp, index, type, size, data);
> +
> +error1:
>  	if (ret == -ENODEV)
>  		rtl_set_unplug(tp);
>  
> @@ -1498,31 +1490,24 @@ static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
>  		if (byen != BYTE_EN_DWORD)
>  			size -= 4;
>  
> -		while (size) {
> -			if (size > limit) {
> -				ret = set_registers(tp, index,
> -						    type | BYTE_EN_DWORD,
> -						    limit, data);
> -				if (ret < 0)
> -					goto error1;
> -
> -				index += limit;
> -				data += limit;
> -				size -= limit;
> -			} else {
> -				ret = set_registers(tp, index,
> -						    type | BYTE_EN_DWORD,
> -						    size, data);
> -				if (ret < 0)
> -					goto error1;
> -
> -				index += size;
> -				data += size;
> -				size = 0;
> -				break;
> -			}
> +		while (size > limit) {
> +			ret = set_registers(tp, index, type | BYTE_EN_DWORD,
> +					    limit, data);
> +			if (ret < 0)
> +				goto error1;
> +
> +			index += limit;
> +			data += limit;
> +			size -= limit;
>  		}
>  

   And here...

> +		ret = set_registers(tp, index, type | BYTE_EN_DWORD, size, data);
> +		if (ret < 0)
> +			goto error1;
> +
> +		index += size;
> +		data += size;
> +
>  		/* Set the last DWORD */
>  		if (byen != BYTE_EN_DWORD)
>  			ret = set_registers(tp, index, type | byen, 4, data);

MBR, Sergey

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

[David Laight]

On Sun, 23 Aug 2026 10:52:12 +0300
Sergey Shtylyov <s.shtylyov@auroraos.dev> wrote:

> On 8/22/26 11:21 PM, Sergey Shtylyov 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.

David

> 
> > +	ret = get_registers(tp, index, type, size, data);
> > +
> > +error1:
> >  	if (ret == -ENODEV)
> >  		rtl_set_unplug(tp);
> >  
> > @@ -1498,31 +1490,24 @@ static int generic_ocp_write(struct r8152 *tp, u16 index, u16 byteen,
> >  		if (byen != BYTE_EN_DWORD)
> >  			size -= 4;
> >  
> > -		while (size) {
> > -			if (size > limit) {
> > -				ret = set_registers(tp, index,
> > -						    type | BYTE_EN_DWORD,
> > -						    limit, data);
> > -				if (ret < 0)
> > -					goto error1;
> > -
> > -				index += limit;
> > -				data += limit;
> > -				size -= limit;
> > -			} else {
> > -				ret = set_registers(tp, index,
> > -						    type | BYTE_EN_DWORD,
> > -						    size, data);
> > -				if (ret < 0)
> > -					goto error1;
> > -
> > -				index += size;
> > -				data += size;
> > -				size = 0;
> > -				break;
> > -			}
> > +		while (size > limit) {
> > +			ret = set_registers(tp, index, type | BYTE_EN_DWORD,
> > +					    limit, data);
> > +			if (ret < 0)
> > +				goto error1;
> > +
> > +			index += limit;
> > +			data += limit;
> > +			size -= limit;
> >  		}
> >    
> 
>    And here...
> 
> > +		ret = set_registers(tp, index, type | BYTE_EN_DWORD, size, data);
> > +		if (ret < 0)
> > +			goto error1;
> > +
> > +		index += size;
> > +		data += size;
> > +
> >  		/* Set the last DWORD */
> >  		if (byen != BYTE_EN_DWORD)
> >  			ret = set_registers(tp, index, type | byen, 4, data);  
> 
> MBR, Sergey
> 
> 

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

[David Laight]

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
> 
> 

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

[Sergey Shtylyov]

On 8/23/26 12:51 PM, David Laight 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.

   Ah, indeed, I didn't realize this check is also "strange": the
code of the last iteration gets always executed, even if !(size %
limit)... Anyway, the current code is safe against !size on entry
and mine is not. I think I should play safe and keep that behavior...

[...]

MBR, Sergey