On 24.04.14 02:45, Bob Beck wrote: > Hi Dirk, I'm not fond of this because you've included all the various > BLAHWOOF_free functions in the bag.
I agree that this creepy macro hell must die. It makes auditing the software nearly impossible. What they basically have done is to put OO-lipstick on a "I normally only do assembly"-pig. Whatever language they believe this cruft was written in, it definitely is not C. Hell, they even need an extra _PERL_ pre-processor to expand all their macros. Once you're stuck like that you should accept that the programming language you chose is not the right for your job – or your skills. I've found nearly a hundred different _free() functions that all behave slightly different even thought they're supposed to be abstracted away by the sk_* macros. It took me a while to figure out, which ones are safe to call with a null pointer (I strongly believe, they all should not crash or warn) and none of these – lets call them callback functions – has had an appropriate set of calling conventions in the first place. So I think that the right way to move on is to enforce those conventions and fix the class instances to those creepy objects, if they do not adhere. > For the moment I would prefer you only deal with the actual calls to > intrinsic free, and leave the others > alone until we can go audit them. Some of these may end up becoming > real free's when they grow up. Maybe we take a subset of definitely internal free() functions like BIO_free, BIO_free_all, BN_free, SSL_free and SSL_CTX_free and include them in this patch and look at the more abstracted ones later. I've found scary constructs like - if ((ret != NULL) && ((a == NULL) || (*a != ret))) + if ((a == NULL) || (*a != ret)) that can by written even more understandable without the redundant checks everywhere. What do you think? erdgeist
