lkml.org 
[lkml]   [2015]   [Sep]   [15]   [last100]   RSS Feed
Views: [wrap][no wrap]   [headers]  [forward] 
 
Messages in this thread
    /
    SubjectRe: First kernel patch (optimization)
    From
    Date
    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



    \
     
     \ /
      Last update: 2015-09-16 00:21    [W:2.779 / U:0.008 seconds]
    ©2003-2020 Jasper Spaans|hosted at Digital Ocean and TransIP|Read the blog|Advertise on this site