> The correct way to do this is to call GENERAL_NAMES_free. This is because sk_GENERAL_NAME_free frees only the containing structure, whereas GENERAL_NAMES_free frees the structure AND its items.
And later:
> Here, the code assumes that a return value that is negative or zero indicates failure, and ‘buf’ is not initialized, and needs not to be freed. But in fact, this is ONLY the case if ASN1_STRING_to_UTF8 returns a negative value. A return value 0 simply means a string of length 0, but memory is nonetheless allocated, so there are memory leaks here as well.
It mirrors my experience working with OpenSSL: you have to quadruple check each function invocation with the docs to make sure you got it right. You're never sure at a glance what's an input or an output parameter, what needs to be freed and how you're supposed to free it. What's the return value in case of error? 0? -1? <0? <=0? And then you have the macro soup with their STACK_OF "abstraction" (in the leakiest sense of the word) that just serves to make the code harder to reason about and is a huge pain when you want to create OpenSSL bindings for an other language.
Let me be clear: I don't blame the OpenSSL devs in any way. It's free, it's open source, they don't get a ton of money for that. Instead I blame all the big corporations who use OpenSSL "as-is", directly or indirectly, and don't invest some money to improve that mess. Maybe after a couple more Heartbleed-like vulnerabilities they'll take it a little more seriously.