Rami has been attempting to make the request on my behalf. I’m joining to hopefully help clarify the issue.
The code snippet posted needs a bit of context. We have an application that in written in C# that calls into registered callbacks functions provided in a C++ shared library (dll). These callbacks must follow a particular signature (short of a very big refactoring). The context is as follows:
The signature provides for returning a boolean and an output parameter. We cannot allow exceptions to cross the callback boundary because we’ll be transiting both a shared library boundary (which may have linked to it a different C++ run time - don’t ask) as well as a language boundary (C++ into managed C++ into C#). Thus, we have to catch all expected exceptions. In this case, std::bad_alloc is an expected exception. The others above are provided as examples.
The code below the exception handlers is common and does the required cleanup. The exception blocks are there only to satisfy our mandatory coding requirements wherein, if we catch exceptions, each expected exception must be explicitly caught. It makes not sense to replicate the common cleanup code in each of the catch blocks. The general pattern we’ve agreed upon is as follows:
bool mycallback(/*input and output parameters */)
bool success = false;
// any variables that may need cleanup later
// do stuff that may throw exceptions
success = true;
// log in each catch block
// log in each catch block
// do common required cleanup based on above variables
The calling program doesn’t care why the function failed. In this case, if a bad_alloc happens, we have nothing useful that we can do based on the calling requirements & expectations.
Note that if the allocation failure happens in a callback, and we suppress it, then it will happen later in the main code where we can handle it gracefully. The restriction is that this function must meet callback signatures and functional expectations.
Also, the rule to allow a comment in a catch block to satisfy SonarQube is present for C#. Why is it not acceptable for C++ as well? We’d prefer to use a comment to explain what’s going on rather than a // NOSONAR comment that doesn’t convey any information.
Please note that we’ve never seen the bad_alloc exception occur for any reason, either in a callback or in the rest of the codebase (we have millions of installations that have been running for years and the processes are fairly long-lived). Its explicit presence is an artifact of the need to not allow exception propagation combined with our coding standard. So please don’t infer from the fact that the catch is there to mean that the exception actually happens during execution.
The suggestion to deallocate a pre-allocated chunk of memory on allocation failure only postpones the problem. The only place where it would make sense to use it is in the logger itself to ensure that a log line is output. We’re open to that, but it doesn’t actually solve the allocation problem (especially when threads are involved, there is no guarantee of who gets the freed memory). It would allow us to bypass this particular issue because we could then log in the bad_alloc catch, but we do have cases where we can’t use loggers as well. We’re still stuck using // NOSONAR for those cases.