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.
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.
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.
Davidbrcz|9 days ago
I'm assuming that using defer would have prevented the gotos in the first case, and the bug.
anilakar|9 days ago
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
mort96|9 days ago
Using defer, the code would be:
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.