Thread (13 messages) flat view 13 messages, 4 authors, 11d ago

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

From: David Laight <hidden>
Date: 2026-08-23 09:51:14
Also in: linux-usb

On Sun, 23 Aug 2026 11:20:30 +0300
Sergey Shtylyov [off-list ref] wrote:
On 8/23/26 11:11 AM, David Laight wrote:
[...]
quoted
quoted
quoted
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 <redacted>  
   Actually, scratch this patch -- it's not entirely correct... :-/

[...]
 
quoted
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
quoted
David  
[...]

MBR, Sergey
  
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help