> On 19 Feb 2015, at 4:19 am, Gabriela Gibson <[email protected]> wrote: > > Hi, > > Reading through the source, I see that a lot of mallocs() do not have a > > [[ > if(foo == NULL) { > perror("Foo: Out of memory.\n"); > return _exit(EXIT_FAILURE); > } > ]]
Strictly speaking, one is supposed to check the result of every malloc call, and handle that in a graceful manner - as you’ve suggested. This, however, comes with costs - and those are very significant costs - in terms of having a lot of extra code to do this checking, for what I would argue is minimal benefit. I just did a quick grep through the sources for the main functions that call the C library’s memory allocator, and came up with these counts: malloc 37 calloc 73 realloc 8 strdup 194 So this is a total of 312 places in the code you’d need to add the above. Most C code I’ve seen either ignores failures of malloc and similar functions, or alternatively wraps them in functions which do the exit check themselves, typically called xmalloc, xcalloc, xrealloc, xstrdup, etc. Reading Dennis’s email, this is the same as what he’s suggested. So we could do this, and cause a process exit whenever memory allocation fails. My question is whether this is really the best way to handle the problem, or if it needs to be handled at all (in a library). DocFormats itself is not something that runs standalone; it’s always embedded in an application (dftest and dfconvert are two examples, but the intention is there’d be larger examples). So I think that if we’re going to add error handling, it would be much better to delegate that to the application itself, rather than taking the decision to terminate the process ourselves. Currently if DocFormats experiences a memory allocation failure, it crashes, and the process exits. With a malloc (and similar functions) check that calls exit, then wehn a memory allocation fails, the process exits. Both are equally undesirable situations from an application’s point of view, so I don’t think we buy anything from either the explicit exit call like above, or wrapper functions that do this. On a desktop OS, an application could launch a sub-process, either via fork() on Unix or CreateProcess on Windows, so that a fatal error leading to a crash or exit call during the conversion would not affect the application itself. Unfortunately some platforms (I’m looking at you, iOS) don’t allow fork(), and everything has to be done within the one process. About a year ago I ported TeX to iOS, and it had calls all over the place that would simply exit if it had a problem, under the (usually correct) assumption that it’d be running as a sub-process, and the caller could just continue on normally. But if you do this in an iOS app, then the user is unceremoniously dumped back to the home screen, without any indication as to what has gone wrong. In practice, memory allocation failures are extremely unlikely, but if we do want to handle them gracefully then I argue we should do it properly - calling exit() gives you practically no benefit over simply crashing. A way to handle this that avoids either case is to use exception handling - setjmp() and longjmp() in C. This is how I handled the case of TeX’s exit() calls; I would simply call setjmp() before the start of the TeX run, and then replace the calls to exit() with longjmp(). The next thing to address i cleanup. If a malloc failure occurs deep in a conversion run, and we jump out to an exception handler, we should free all memory that was allocated during that run. This means wrapping malloc/calloc/realloc/strdup and any other functions in the C library, and using our own data structure to keep track of what has been allocated (and also to update this data structure when memory is freed). Then, if an error occurs, after we return to the call site of setjmp() and go into the exception handling block, we go through this data structure and call free() on all the blocks of memory that were allocated by DocFormats, so the application can then show an error message and then continue operating. TL;DR - calling exit() doesn’t give any practical benefit over simply crashing; if we want to handle out-of-memory errors we need should do so cleanly in a way that allows the application using the library to continue running, and that means tracking & subsequently freeing all memory allocated by DocFormats, to avoid memory leaks and further out-of-memory errors. — Dr Peter M. Kelly [email protected] PGP key: http://www.kellypmk.net/pgp-key <http://www.kellypmk.net/pgp-key> (fingerprint 5435 6718 59F0 DD1F BFA0 5E46 2523 BAA1 44AE 2966)
