Messages in this thread | | | Subject | Re: First kernel patch (optimization) | From | Alexander Duyck <> | Date | Tue, 15 Sep 2015 14:57:05 -0700 |
| |
On 09/15/2015 12:52 PM, Eric Curtin wrote: > My first kernel patch, hope I did everything correctly! Instead of calling strlen on every iteration of the for loop, just call it once instead and store in a variable. > > Signed-off-by: Eric Curtin <ericcurtin17@gmail.com> > > diff --git a/tools/usb/usbip/src/usbip_detach.c b/tools/usb/usbip/src/usbip_detach.c > index 05c6d15..9db9d21 100644 > --- a/tools/usb/usbip/src/usbip_detach.c > +++ b/tools/usb/usbip/src/usbip_detach.c > @@ -47,7 +47,9 @@ static int detach_port(char *port) > uint8_t portnum; > char path[PATH_MAX+1]; > > - for (unsigned int i = 0; i < strlen(port); i++) > + unsigned int port_len = strlen(port); > + > + for (unsigned int i = 0; i < port_len; i++) > if (!isdigit(port[i])) { > err("invalid port %s", port); > return -1; >
You should probably run this through scripts/checkpatch.pl as I don't think declaring i inside the loop is consistent with the kernel coding standard. Also you don't want the space between port_len and the declaration of path.
Also you might want to consider just running the loop from port_len down to 0 doing something like: while (port_len) { if (!isdigit(port[--port_len])) { err("invalid port %s", port); return -1;
The advantage to running the loop backwards is that you have to carry one less variable which means one less register to mess with in the final compiled code.
- Alex
| |