On 06/08/2015 05:26 PM, Rasmus Villemoes wrote:
Part of the disassembly of do_blk_trace_setup:

     231b:       e8 00 00 00 00          callq  2320 <do_blk_trace_setup+0x50>
                         231c: R_X86_64_PC32     strlen+0xfffffffffffffffc
     2320:       eb 0a                   jmp    232c <do_blk_trace_setup+0x5c>
     2322:       66 0f 1f 44 00 00       nopw   0x0(%rax,%rax,1)
     2328:       48 83 c3 01             add    $0x1,%rbx
     232c:       48 39 d8                cmp    %rbx,%rax
     232f:       76 47                   jbe    2378 <do_blk_trace_setup+0xa8>
     2331:       41 80 3c 1c 2f          cmpb   $0x2f,(%r12,%rbx,1)
     2336:       75 f0                   jne    2328 <do_blk_trace_setup+0x58>
     2338:       41 c6 04 1c 5f          movb   $0x5f,(%r12,%rbx,1)
     233d:       4c 89 e7                mov    %r12,%rdi
     2340:       e8 00 00 00 00          callq  2345 <do_blk_trace_setup+0x75>
                         2341: R_X86_64_PC32     strlen+0xfffffffffffffffc
     2345:       eb e1                   jmp    2328 <do_blk_trace_setup+0x58>

Yep, that's right: gcc isn't smart enough to realize that replacing
'/' by '_' cannot change the strlen(), so we call it again and again
(at least when a '/' is found). Even if gcc were that smart, this
construction would still loop over the string twice, once for the
initial strlen() call and then the open-coded loop.

Let's simply use strreplace() instead.

Patch looks fine to me, but there's no strreplace in in Linus' tree. Dependencies like that should be noted in the patch. Please send followup patches like this when the main patch is in Linus tree, and I'd be happy to apply it.

--
Jens Axboe

--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/

Reply via email to