CODEWITHSUNDEEP
c++destructorunions
masT
masT

Reputation: 850

C++: Union Destructor

A union is a user-defined data or class type that, at any given time, contains only one object from its list of members. Suppose all the possible candidate members are needed to be allocated dynamically. For Eg.

// Union Destructor
#include <string>
using namespace std;

union Person
{
private:
    char* szName;
    char* szJobTitle;
public:
    Person() : szName (nullptr), szJobTitle (nullptr) {}
    Person (const string& strName, const string& strJob)
    {
        szName = new char[strName.size()];
        strcpy (szName, strName.c_str());

        szJobTitle = new char [strJob.size()];
        strcpy (szJobTitle, strJob.c_str());    // obvious, both fields points at same location i.e. szJobTitle
    }
    ~Person()   // Visual Studio 2010 shows that both szName and szJobTitle
    {           // points to same location.
        if (szName) {
            delete[] szName;     // Program crashes here.
            szName = nullptr;  // to avoid deleting already deleted location(!)
        }
        if (szJobTitle)
            delete[] szJobTitle;
    }
};

int main()
{
    Person you ("your_name", "your_jobTitle");
    return 0;
}

Above program get crashed at 1st delete statement in ~Person (at moment when szName contains valid memory location, WHY?).

What will be the correct implementation for the destructor?

Same way, how to destruct a class object, if my class contains an union member (how to wrtie destructor for class including a Union)?

Upvotes: 4

Views: 5191

Answers (4)

Jan Herrmann
Jan Herrmann

Reputation: 2767

You can only use one member of the union at a time because they share the same memory. In the constructor, however, you initialize both members, which overwrites each other, and then in the destructor you end up releasing it two times. You're trying to use it as if it were a struct (based on the names of the fields you need to use a struct).

Nevertheless, if you need a union then you probably need a struct as a kind of envelope which has some id representing the member being used, as well as a constructor and a destructor that handles the resources.

Also - your arrays are too small. size() returns the number of characters, but if you use char* as your string type then you need space for the null-character (\0) to handle termination.

If you need unions, try using Boost.Variant. It is a lot easier to use than normal unions.

Upvotes: 4

utnapistim
utnapistim

Reputation: 27365

Above program get crashed at 1st delete statement in ~Person (at moment when szName contains valid memory location, WHY?).

I don't have a compiler with me (or time to compile your code) but (apart from the problems addressed by Nawaz) I would guess it's because you treat the union members as class members. In your union, szName and szJobTitle should be looked like two variables with the same address:

Person (const string& strName, const string& strJob)
{
    szName = new char[strName.size()];
    strcpy (szName, strName.c_str());

    szJobTitle = new char [strJob.size()]; // this creates memory leak (1)
    strcpy (szJobTitle, strJob.c_str());
}

You get a memory leak because you allocate new memory and place it in szJobTitle. &szJobTitle uses the same memory location as &szName, so with the assignment in line (1) you loose the address allocated in szName. If szName and szJobTitle were of different types (with non-matching memory footprints), setting szJobTitle would also corrupt (or only partially overwrite szTitle).

What will be the correct implementation for the destructor?

I think you do not have enough details for implementing the destructor. Look into the concept of discriminated unions in C++ to see how to implement this correctly. Normally your union members should manage their own memory (use std::string, not char*) and then your destructor would only delete what was allocated (but you will have to call it explicitly).

Same way, how to destruct a class object, if my class contains an union member (how to wrtie destructor for class including a Union)?

Again, look at discriminated unions. It is basically the association of a union and an enum, where the enum maps to the members of the union and is set to specify which of the members of the union was set.

Upvotes: 1

Sarfaraz Nawaz
Sarfaraz Nawaz

Reputation: 361392

You're using delete when you should use delete [], because you have used new [], not new.

Change these:

delete szName;
delete szJobTitle;

to these:

delete [] szName;
delete [] szJobTitle;

By the way, the if condition in the destructor is pointless. I mean, if a pointer is nullptr, then it is safe to write delete ptr;, that is,

A *ptr = nullptr;
delete ptr; //Okay! No need to ensure ptr is non-null

Apart from that, you're violating rule of three (or five, in C++11):

Implement them.

Upvotes: 2

Angew is no longer proud of SO
Angew is no longer proud of SO

Reputation: 171127

You don't respect that new-delete pairing: new pairs with delete, new[] pairs with delete[]. You're doing new[], but calling delete; that's incompatible.

As a side note, the constructor has a memory leak: the memory assigned to szName is never freed once that pointer is overwritten by the assignment to szJobTitle).

And as this is C++, you should generally be using std::string instead of char* for strings.

Upvotes: 1

Related Questions