[bug report] [PATCH] dvb: b2c2/flexcop driver refactoring part 2: add modular Flexcop driver

[bug report] [PATCH] dvb: b2c2/flexcop driver refactoring part 2: add modular Flexcop driver

[Dan Carpenter]

[ This patch is 19 years old now...  :P  Sorry!  - dan ]

Hello Johannes Stezenbach,

Commit 2add87a95068 ("[PATCH] dvb: b2c2/flexcop driver refactoring
part 2: add modular Flexcop driver") from May 16, 2005 (linux-next),
leads to the following Smatch static checker warning:

	drivers/media/usb/b2c2/flexcop-usb.c:199 flexcop_usb_memory_req()
	warn: iterator 'i' not incremented

drivers/media/usb/b2c2/flexcop-usb.c
    178 static int flexcop_usb_memory_req(struct flexcop_usb *fc_usb,
    179                 flexcop_usb_request_t req, flexcop_usb_mem_page_t page_start,
    180                 u32 addr, int extended, u8 *buf, u32 len)
    181 {
    182         int i, ret = 0;
    183         u16 wMax;
    184         u32 pagechunk = 0;
    185 
    186         switch (req) {
    187         case B2C2_USB_READ_V8_MEM:
    188                 wMax = USB_MEM_READ_MAX;
    189                 break;
    190         case B2C2_USB_WRITE_V8_MEM:
    191                 wMax = USB_MEM_WRITE_MAX;
    192                 break;
    193         case B2C2_USB_FLASH_BLOCK:
    194                 wMax = USB_FLASH_MAX;
    195                 break;
    196         default:
    197                 return -EINVAL;
    198         }
--> 199         for (i = 0; i < len;) {

No i++.

    200                 pagechunk =
    201                         wMax < bytes_left_to_read_on_page(addr, len) ?
    202                                 wMax :
    203                                 bytes_left_to_read_on_page(addr, len);
    204                 deb_info("%x\n",
    205                         (addr & V8_MEMORY_PAGE_MASK) |
    206                                 (V8_MEMORY_EXTENDED*extended));
    207 
    208                 ret = flexcop_usb_v8_memory_req(fc_usb, req,
    209                         page_start + (addr / V8_MEMORY_PAGE_SIZE),
    210                         (addr & V8_MEMORY_PAGE_MASK) |
    211                                 (V8_MEMORY_EXTENDED*extended),
    212                         &buf[i], pagechunk);
                                ^^^^^^^^
I think adding an i++ doesn't make sense.  Are we really writing a byte
at a time?

    213 
    214                 if (ret < 0)
    215                         return ret;
    216                 addr += pagechunk;
    217                 len -= pagechunk;
    218         }
    219         return 0;
    220 }

regards,
dan carpenter

Re: [bug report] [PATCH] dvb: b2c2/flexcop driver refactoring part 2: add modular Flexcop driver

[Johannes Stezenbach]

Hi Dan,

On Sat, May 04, 2024 at 02:24:21PM +0300, Dan Carpenter wrote:
> [ This patch is 19 years old now...  :P  Sorry!  - dan ]
> 
> Commit 2add87a95068 ("[PATCH] dvb: b2c2/flexcop driver refactoring
> part 2: add modular Flexcop driver") from May 16, 2005 (linux-next),
> leads to the following Smatch static checker warning:

I think the patches were from Patrick and misattributed because
I was too stupid to operate git correctly at the time.

> 	drivers/media/usb/b2c2/flexcop-usb.c:199 flexcop_usb_memory_req()
> 	warn: iterator 'i' not incremented

> --> 199         for (i = 0; i < len;) {
> 
> No i++.

>     208                 ret = flexcop_usb_v8_memory_req(fc_usb, req,
>     209                         page_start + (addr / V8_MEMORY_PAGE_SIZE),
>     210                         (addr & V8_MEMORY_PAGE_MASK) |
>     211                                 (V8_MEMORY_EXTENDED*extended),
>     212                         &buf[i], pagechunk);
>                                 ^^^^^^^^
> I think adding an i++ doesn't make sense.  Are we really writing a byte
> at a time?
C> 
>     213 
>     214                 if (ret < 0)
>     215                         return ret;
>     216                 addr += pagechunk;
>     217                 len -= pagechunk;
>     218         }

The loop is weird, but I guess it worked because the len -= pagechunk
would have terminated the loop and supposedly there was only one
iteration ever. I doubt anyone has hardware to test it, so don't
change it. Well, I certainly won't touch it, you can do it if you want.


Johannes

Re: [bug report] [PATCH] dvb: b2c2/flexcop driver refactoring part 2: add modular Flexcop driver

[Dan Carpenter]

On Sat, May 04, 2024 at 10:24:47PM +0200, Johannes Stezenbach wrote:
> Hi Dan,
> 
> On Sat, May 04, 2024 at 02:24:21PM +0300, Dan Carpenter wrote:
> > [ This patch is 19 years old now...  :P  Sorry!  - dan ]
> > 
> > Commit 2add87a95068 ("[PATCH] dvb: b2c2/flexcop driver refactoring
> > part 2: add modular Flexcop driver") from May 16, 2005 (linux-next),
> > leads to the following Smatch static checker warning:
> 
> I think the patches were from Patrick and misattributed because
> I was too stupid to operate git correctly at the time.
> 
> > 	drivers/media/usb/b2c2/flexcop-usb.c:199 flexcop_usb_memory_req()
> > 	warn: iterator 'i' not incremented
> 
> > --> 199         for (i = 0; i < len;) {
> > 
> > No i++.
> 
> >     208                 ret = flexcop_usb_v8_memory_req(fc_usb, req,
> >     209                         page_start + (addr / V8_MEMORY_PAGE_SIZE),
> >     210                         (addr & V8_MEMORY_PAGE_MASK) |
> >     211                                 (V8_MEMORY_EXTENDED*extended),
> >     212                         &buf[i], pagechunk);
> >                                 ^^^^^^^^
> > I think adding an i++ doesn't make sense.  Are we really writing a byte
> > at a time?
> C> 
> >     213 
> >     214                 if (ret < 0)
> >     215                         return ret;
> >     216                 addr += pagechunk;
> >     217                 len -= pagechunk;
> >     218         }
> 
> The loop is weird, but I guess it worked because the len -= pagechunk
> would have terminated the loop and supposedly there was only one
> iteration ever. I doubt anyone has hardware to test it, so don't
> change it. Well, I certainly won't touch it, you can do it if you want.

Nah.  It's fine.  If you don't know, then I for sure don't know.

regards,
dan carpenter