1. 11

  2. 11

    This is quite disappointing, his solution for each point is basically “just get it right!”

    Here are some actual solutions for the problems he brings up:

    Not freeing memory after allocation:

    Try to avoid the “free everything individually” pattern where possible because it’s the easiest to get wrong. Use RAII if you like. Try to centralise your resource management higher up the call stack so leaf code does not call malloc/free, people coming from other languages tend to do that because it’s fine if you have a GC, but you can make big messes for yourself in C. Use easier allocation strategies like “free everything allocated after this point when this statement falls out of scope”, which works very well for temp scratch space allocations.

    All of your allocations should go through memory managers that check for leaks. The least intrusive way to do it is by keeping a hashtable that maps from the pointer to some info struct, which at least contains file and line of the allocation.

    Freeing already freed memory (double-freeing):

    Largely mitigated by what I said before. I have no specific suggestions for this because in practice I never have problems with it.

    Invalid memory access, either reading or writing:

    I don’t have any recommendations for NULL pointers, but stuffing asserts/NULL checks all over the place is a signal you have no idea what your code is doing. TBH it’s not really been a problem for me.

    For uninitialised data, making all your allocators call memset( 0 ) is a good start (try to make your classes have some valid default state when they’re all 0), beyond that it’s not really a problem because it’s pretty easy to catch. Use RAII if you like.

    Also lobby the C++ committee for something like #pragma primitive_types_initialise_to_zero_unless_you_tell_them_not_to.

    Buffer-overflow, either reading or writing:

    Implement sane primitives, such as:

    template< typename T >
    struct array {
        T * elems;
        size_t n;
        T & operator[]( size_t i ) { ASSERT( i < n ); return elems[ i ]; }

    Replace any usage of pointer + size with that class. You will probably want StaticArray< size_t, T >, DynamicArray, array2d, etc too.

    You’ll also want a sane string class, a sane string parsing library (Lua’s patterns library is very very good), and a sane stream writer/parser class. Use these classes instead of C strings/pointers + sizes.

    1. 2

      As for your first solution, I would first check to see if the runtime can do the checks for you. I know that the GNU C library can do more checks if you set certain environment variables, and using valgrind is wonderful if you have access to it. Check first before writing your own memory wrapper (that is what lead to Heartbleed, by the way).

      Second, pick a better value to initialize memory with than just 0. If you really want to help with debugging on the x86 platform, I suggest using the value 0xCC. As a signed quantity, it’s a large negative value that is easy to spot. As an unsigned quantity, it’s a huge number. As a character, it’s invalid ASCII (but it is a valid UTF-8 initial byte character, but two 0xCC’s in a row is invalid UTF-8). As a pointer, it’s probably an invalid pointer (so you’ll get a SIGSEGV most likely) and if it’s accidentally executed, it’s the INT3 debugging instruction.

      I don’t really have any good solutions to string handling in C other than “don’t do that!” Which is why I use Lua if possible.