Search code examples
c++gdbcoredump

segmentation fault when allocating


I am getting a segmentation fault and would appreciate help trying to understand what I have done incorrectly.

After a while of my code running I have received a coredump: Program terminated with signal SIGSEGV, Segmentation fault.

struct config {
    int a;
    int b;
    int c;
    int d;
};

static config* configuration_table = NULL;

void Album::caller() {
   
    replay(2, configuration_table, 0, false);
}

int Album::replay(int count, config*& config_tbl, unsigned int& config_sz, bool send) {
    int num_elems =0;

    ...
    ...
    ...

    err = get_num_elems(&num_elems);
    if (err) {
        return -1;
    }

    if (!num_elems) {
        return -1;
    }

    if (config_tbl) {
        delete[] config_tbl;
        config_tbl = NULL;
        config_sz = 0;
    }
    
    config_tbl = new config[num_elems];
    if (!config_tbl) {
        return -1;
    }
}

gdb:

4 0xf9abd319 in Album::replay (this=0xa693955, count=2, config_tbl=@0xf9d4299e: 0x0, config_sz=@0xa3039fe: 0, send=false)

from the backtrace it appears the problem is with malloc that comes from the operator "new" from "config_tbl = new config[num_elems];". num_elems has size 15 when I debugged it.

I don't understand what I have done incorrectly. Is is possible that the system is out of memory?


Solution

  • Your code assumes that config_tbl is properly initialized. As you insist that the code you posted is all that is relevant, I assume it is not properly initialized.

    Consider this simplified example:

    void foo( int* ptr) {
        if (ptr) delete ptr;
    }
    

    This is fine if ptr is either nullptr or a valid pointer (ie it points to an int created via new). Correct usage is either

    foo(nullptr);
    

    or

    int* x = new int;
    foo(x);
    

    However, it is too easy to make foo go havoc:

    int* x;
    foo(x);      // UNDEFINED BEHAVIOR
    int y;
    foo(&y);     // BOOM
    

    Do not use pointers for dynamic arrays in the first place. Use std::vector instead.

    PS: Note that while foo is utterly bad code, it isn't wrong. It is the calling code that can cause a crash.