deque push back default constructor not called

Question

I implemented a class following the rule of three, and I am getting a crash. Upon debugging I have came to the conclusion that in the copy constructor line number 51, the pointer oQueue is not NULL due to some internal logic of deque and hence the constructor tries to delete the memory and fails.

I then read somewhere that I should not check for NULLness of oQueue in copy constructor because it might not be initialized in the constructor. Should not oQueue be always initialized to NULL due to the default constructor?

    #include <iostream>
    #include <deque>
    #include <cstdlib>
    #define LENGTH 128

    typedef struct tDataStruct
    {

    char strA[LENGTH];

    char strB[LENGTH];
    int nNumberOfSignals;
    double* oQueue;

    tDataStruct()
    {
        nNumberOfSignals = 0;
        oQueue = NULL;
        memset(strA, 0, LENGTH);
        memset(strB, 0, LENGTH);
    }

    ~tDataStruct()
    {
        if (NULL != oQueue)
        {
            delete[] oQueue;
            oQueue = NULL;
        }
    }

    tDataStruct(const tDataStruct& other) // copy constructor
    {
        if (this != &other)
        {
            *this = other;
        }

    }
    tDataStruct& operator=(const tDataStruct& other) // copy assignment
    {
        if (this == &other)
        {
            return *this;
        }
        strncpy_s(strA, other.strA, LENGTH);
        strncpy_s(strB, other.strB, LENGTH);
        nNumberOfSignals = other.nNumberOfSignals;
        if (NULL != oQueue)
        {
            delete[] oQueue;
            oQueue = NULL;
        }
        if (other.nNumberOfSignals > 0)
        {
            //memcpy(oQueue, other.oQueue, nNumberOfSignals);
        }
        return *this;
    }
    } tDataStruct;


    int main()
    {
        tDataStruct tData;

        std::deque<tDataStruct> fifo;

        fifo.push_back(tData);
    }

Are you expecting the default constructor to be called before the copy constructor? That's not the case. — , Oct 11 '18 at 06:00
The usual way to write a copy constructor is to implement it fully, and not call the assignment operator at all. Then the assignment operator is easily implemented by [copy / swap](https://stackoverflow.com/questions/3279543/what-is-the-copy-and-swap-idiom). Right now, your assignment operator has many holes in it. It is far easier to just write the copy constructor as if there is no assignment operator, then come back and implement the assignment operator using the already mentioned copy/swap technique. — PaulMcKenzie, Oct 11 '18 at 06:38
I have got everything working, I just asked this questions to increase my knowledge about how things should be and improve my concepts. — Daniyal Yasin, Oct 11 '18 at 06:40
@DaniyalYasin You say you have everything working, but given your class, why do you even need a user-defined copy constructor or assignment operator? You don't use the `oQueue` pointer member at all (no dynamic memory allocation is occurring), so it reduces down to a class that works without having to write a copy constructor or assignment operator. — PaulMcKenzie, Oct 11 '18 at 06:44
This is redacted code. I redacted a large portion to create a minimum verifiable example. I used this redacted version to ask another question yesterday on which I was stuck, afterwards I got stuck on this problem. But before I had permission to ask a question (90 minutes), i found the answer so just asked for a conceptual clarification. — Daniyal Yasin, Oct 11 '18 at 06:54

score 2 · Accepted Answer · answered Oct 11 '18 at 06:05

Your implementation of the copy constructor invokes undefined behavior since the member variables of the object being constructed have not been initialized.

You can use the default constructor to initialize the member variables first to get predictable behavior.

tDataStruct(const tDataStruct& other) : tDataStruct()
{
   *this = other;
}

deque push back default constructor not called

1 Answers1