Comment 12 for bug 1990903

Revision history for this message
dwmw2 (dwmw2) wrote :

> This check for headers_array in the loop’s termination condition is only in the upstream code, not in the one that was in Ubuntu at the time I reported the bug and submitted the patch.

Yes. That's why I'm pointing out the difference and suggesting that we reconcile the code.

> When I made the patch I decided to put a proper if-clause there because it’s easier to understand what is going on (the check in the for loop seems like obfuscated programming to me)

Agreed. Happy to accept a patch which cleans that up in the upstream tree. Although...

> and technically it’s even faster to check only once instead of at every loop iteration (even though this minimal performance gain is hardly significant here).

In practice any compiler worth its salt should optimise that away because it'll know that calling free() won't affect the value of headers_array. See in 'objdump -S' output how the loop only jump back from 0x8d98 to 0x9d88, adding 8 to the pointer every time until it reaches the precalculated end value.

        free(cookie_array);
    8d6a: 48 8b 3c 24 mov (%rsp),%rdi
    8d6e: e8 ed da ff ff call 6860 <free@plt>
        for (i = 0; headers_array && i < 2 * (num_headers + 1); i++) {
    8d73: 4d 85 ff test %r15,%r15
    8d76: 74 22 je 8d9a <cookie_cb+0x25a>
    8d78: 43 8d 44 24 01 lea 0x1(%r12,%r12,1),%eax
    8d7d: 4c 89 fd mov %r15,%rbp
    8d80: 4d 8d 64 c7 08 lea 0x8(%r15,%rax,8),%r12
    8d85: 0f 1f 00 nopl (%rax)
                free(headers_array[i]);
    8d88: 48 8b 7d 00 mov 0x0(%rbp),%rdi
        for (i = 0; headers_array && i < 2 * (num_headers + 1); i++) {
    8d8c: 48 83 c5 08 add $0x8,%rbp
                free(headers_array[i]);
    8d90: e8 cb da ff ff call 6860 <free@plt>
        for (i = 0; headers_array && i < 2 * (num_headers + 1); i++) {
    8d95: 49 39 ec cmp %rbp,%r12
    8d98: 75 ee jne 8d88 <cookie_cb+0x248>
        free(headers_array);
    8d9a: 4c 89 ff mov %r15,%rdi
    8d9d: e8 be da ff ff call 6860 <free@plt>