top | item 47085563

(no title)

uecker | 9 days ago

I do not see how defer would have helped in this case.

discuss

order

Davidbrcz|9 days ago

People manually doing resource cleanup by using goto.

I'm assuming that using defer would have prevented the gotos in the first case, and the bug.

anilakar|9 days ago

To be fair, there were multiple wrongs in that piece of code: avoiding typing with the forward goto cleanup pattern; not using braces; not using autoformatting that would have popped out that second goto statement; ignoring compiler warnings and IDE coloring of dead code or not having those warnings enabled in the first place.

C is hard enough as is to get right and every tool and development pattern that helps avoid common pitfalls is welcome.

uecker|9 days ago

I don't see this. The problem was a duplicate "goto fail" statement where the second one caused an incorrect return value to be returned. A duplicate defer statement could directly cause a double free. A duplicate "return err;" statement would have the same problem as the "goto fail" code. Potentially, a defer based solution could eliminate the variable for the return code, but this is not the only way to address this problem.

mort96|9 days ago

Is that true though?

Using defer, the code would be:

    if ((err = SSLHashSHA1.update(&hashCtx, &signedParams)) != 0)
        return err;
        return err;
This has the exact same bug: the function exits with a successful return code as long as the SHA hash update succeeds, skipping further certificate validity checks. The fact that resource cleanup has been relegated to defer so that 'goto fail;' can be replaced with 'return err;' fixes nothing.