Michael Jung mjung@iss.tu-darmstadt.de writes:
If the CSP's CPAcquireContext function fails, which is quite common (e.g. because the user specified key container does not exist), the "temp" variable is still free'd twice.
Yes, the free needs to be moved inside the if, sorry about that.
I'm aware that my original patch was pretty ugly, due to a lot of redundancy, but I think this is due to fact that the CryptAcquireContext function seems not to be very well structured. I've submitted a patch two weeks ago, which basically was a reimplementation of CryptAcquireContext, but which was not committed (http://www.winehq.org/hypermail/wine-patches/2004/07/0213.html). Rob Shearman commented on this patch and felt that it is overkill to add both a helper function and a new struct type. Is this the reason the patch was not applied?
It's overkill and it's ugly. If a function needs such elaborate cleanup it's probably because it's trying to do too many things, and that needs to be fixed; splitting the cleanup part into a separate function only makes things even more confusing.