Why there is something wrong when invoking ctor in another ctor?

Question

Here is a demo code snippet(https://godbolt.org/z/31Tq3r):

 #include<iostream>

class Ctx
{
public:
    enum RUN_MOD
    {
        MOD_RT,
        MOD_NRT,
    };

    Ctx(RUN_MOD runType)
    {
        if(runType == MOD_RT)
        {
            Ctx();
        }
    }

    Ctx()
    {
        m_L = malloc(100);
        std::cout << "set m_L=" << m_L << std::endl;
    }

    void print()
    {
        std::cout <<"print() m_L=" << m_L << std::endl;
    }

private:
    void *m_L;

    const char* const ARG_TYPE_NOT_MATCH = "the type of argument is not match";
    const char* const ARG_NUM_INVALID = "the number of arguments is invalid";
    const char* const STACK_OPS_INPUT_ARG_INVALID = "the input argument passed to the stack ops is invalid";
};


int main()
{
    Ctx ctx(Ctx::RUN_MOD::MOD_RT);

    ctx.print();
}

Here are outputs when invoking the binary program on Ubuntu:

set m_L=0x614c20 
print() m_L=0x400ad0

You see the addresses are not the same. I have set m_L by invoking Ctx::Ctx()(which is called by Ctx::Ctx(RUN_MOD runType)) indeed. I am really confused.

*I am really confused.* -- `if(runType == MOD_RT) { Ctx();}` -- This doesn't do what you think it does. — PaulMcKenzie, Dec 20 '20 at 06:53
I want to reuse a specific ctor. This code snippet is just for demo. — John, Dec 20 '20 at 06:54
The code you posted is what we have to work with, demo or no demo. That line of code does not "chain on" the next constructor. — PaulMcKenzie, Dec 20 '20 at 06:55
Don't use `malloc` in C++. Use `new`. Or even better, use `std::unique_ptr`. That prevents memory leaks like you have now. — JHBonarius, Dec 20 '20 at 06:56
@PaulMcKenzie Do you mean I should not call another ctor in a specific ctor? — John, Dec 20 '20 at 06:59
There's nothing wrong with invoking a constructor within a constructor. It will do exactly what you tell it to do. In your case you tell it to create a new local object and immediately destroy that. (Aldo causing an instant memory leak) — JHBonarius, Dec 20 '20 at 07:01
@John -- C++ is one of the most difficult languages to learn, and to learn it properly requires [peer-reviewed books and other material](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). From what I gather, it looks like you're trying to learn by trial and error (or perusing poor websites or material), which is not the way C++ can be properly learned. — PaulMcKenzie, Dec 20 '20 at 07:34

score 2 · Answer 1 · answered Dec 20 '20 at 07:10

If you are looking for a clean solution you could use a function that does, what the default constructor should do. This way you don't need constructor delegation, which afaik can't be applied conditional and you don't have to mess with placement new.

Minimal example:

class Ctx
{
private:
    void DefaultInit()
    {
        m_L = malloc(100);
        std::cout << "set m_L=" << m_L << std::endl;
    } 

public:
    Ctx(RUN_MOD runType)
    {
        if(runType == MOD_RT)
        {
            this->DefaultInit();
        }
    }

    Ctx()
    {
       this->DefaultInit();
    }
}

score 2 · Accepted Answer · answered Dec 20 '20 at 07:29

Constructor delegation can only be used from inside a constructor’s member initialization list, NOT inside a constructor’s body. This answer shows how to avoid the need to use constructor delegation at all, by creating a common initialization function that multiple constructors can then call as needed.

However, if you did want to use constructor delegation, for whatever reason, then in this case the Ctx() constructor should delegate to the Ctx(RUN_MOD) constructor, not the other way around as you originally attempted, eg:

class Ctx
{
public:
    enum RUN_MOD
    {
        MOD_RT,
        MOD_NRT,
    };

    Ctx(RUN_MOD runType)
    {
        if (runType == MOD_RT)
        {
            m_L = malloc(100);
            std::cout << "set m_L=" << m_L << std::endl;
        }
    }

    Ctx() : Ctx(MOD_RT) // <— here
    {
    }

    void print()
    {
        std::cout << "print() m_L=" << m_L << std::endl;
    }

private:
    void *m_L = nullptr;

    const char* const ARG_TYPE_NOT_MATCH = "the type of argument is not match";
    const char* const ARG_NUM_INVALID = "the number of arguments is invalid";
    const char* const STACK_OPS_INPUT_ARG_INVALID = "the input argument passed to the stack ops is invalid";
};

score 1 · Answer 3 · answered Dec 20 '20 at 07:44

Calling ctor of any class makes an instance of it, in your posted code it is obvious that allocated m_l in Ctx() is not belong to the object constructed by Ctx(RUN_MOD runType)

try below code

void init_m_l()
{
    m_L = malloc(100);
    std::cout << "set m_L=" << m_L << std::endl;
}

Ctx(RUN_MOD runType)
{
    if(runType == MOD_RT)
       init_m_l();
}

Ctx()
{
    init_m_l();
}

John · Answer 4 · 2020-12-20T08:16:13.403

0

I found a solution, here is the code snippet(https://coliru.stacked-crooked.com/a/964309f60f62dbd4, it works, but I am still trying to understand it):

#include<iostream>

class Ctx
{
public:
    enum RUN_MOD
    {
        MOD_RT,
        MOD_NRT,
    };

    Ctx(RUN_MOD runType)
    {
        if(runType == MOD_RT)
        {
            new (this)Ctx();
        }
    }

    Ctx()
    {
        m_L = malloc(100);
        std::cout << "set m_L=" << m_L << std::endl;
    }

    void print()
    {
        std::cout <<"print() m_L=" << m_L << std::endl;
    }

    ~Ctx()
    {
        free(m_L);
    }

    Ctx(const Ctx&) = delete;  //to avoid double free

private:
    void *m_L;

    const char* const ARG_TYPE_NOT_MATCH = "the type of argument is not match";
    const char* const ARG_NUM_INVALID = "the number of arguments is invalid";
    const char* const STACK_OPS_INPUT_ARG_INVALID = "the input argument passed to the stack ops is invalid";
};


int main()
{
    Ctx ctx(Ctx::RUN_MOD::MOD_RT);

    ctx.print();
}

edited Dec 20 '20 at 08:16

answered Dec 20 '20 at 07:02

John

2,963
11
33

Sorry, but this solution is obfuscated C++. – PaulMcKenzie Dec 20 '20 at 07:04
So now you have a dilemma -- how do you deallocate that pointer? Do you use `free`, or do you use `delete`? – PaulMcKenzie Dec 20 '20 at 07:17
I use `free` to release the allocated memory. See the edited answer. – John Dec 20 '20 at 07:26
*I use free* -- No good. [See this](https://stackoverflow.com/questions/3184118/new-delete-malloc-free) – PaulMcKenzie Dec 20 '20 at 07:29

John · Answer 5 · 2020-12-20T07:29:33.067

0

It's a good answer, but the author deleted it. I posted it here for more people to study.

his code:

if(runType == MOD_RT)
{
    Ctx();
}

creates a temporary Ctx() object, and -- that's it. Nothing more, nothing less. That temporary just gets destroyed as soon as that line of code is finished executing.

There are other major issues with the class, such as using malloc, the class leaks memory due to not following the rule of 3, not initializing all the members (which I addressed by setting m_L to nullptr), etc.

edited Dec 20 '20 at 07:29

answered Dec 20 '20 at 07:17

John

2,963
11
33

I deleted it since the answer given by @churill is basically a duplicate. Also, you really shouldn't copy verbatim what was deleted. Others can see the deleted post. – PaulMcKenzie Dec 20 '20 at 07:18
I see, I will delete the duplicated part. But I think your answer explain the issues with the code more clearly. Could you please explain the last paragraph in more detail for me?(i.e " the class leaks memory due to not following the rule of 3, not initializing all the members". I can release the allocated memory by `free` later. So I am confused why you say this.) – John Dec 20 '20 at 07:29
`int main() { Ctx x1; Ctx x2 = x1; }` -- Try that, and be highly disappointed in what gets "freed". You now have a double-free error. That's what the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three) is all about. Go to the **Managing resources** section of that link. – PaulMcKenzie Dec 20 '20 at 07:36
@PaulMcKenzie I think I understand what you mean now after I carefully read the aforementioned section. I should add `Ctx(const Ctx&) = delete;` for my case(the memory which `m_L` poins to is used as a memory pool). Am I right? Thank you for your generous help. – John Dec 20 '20 at 08:23

Why there is something wrong when invoking ctor in another ctor?

5 Answers5