C++ passing struct or object by value

Question

I have this:

enum Units { Pounds, Kilos };

struct Configuration
{
    const Units units;
    const char *name;

    inline Configuration(Units pUnits, char *pName) : units(pUnits)
    {
        name = strdup(pName);
    }

    inline ~Configuration() { free((void *)name); }
};

I was passing one of these to a method like this:

Configuration cc(Kilos, "abc");
cdao->write(cc);

I was getting nasty crashes from this until I tried redefining the method to take a reference:

Configuration cc(Kilos, "abc");
cdao->write(&cc);

And now everything works.

But how could the struct by value be screwing with memory?

Answer 1

The fact that you are using strdup indicates that there is something wrong with your code, and the wrong thing is you don't have a copy constructor. Any time you have a destructor, you almost certainly also need a copy constructor, which will copy the object correctly when you call by value.

To improve your code:

• create a copy constructor and probably an assignment operator which allocate and copy the string correctly

• better yet, get rid of strdup - use a std:;string, in which case you won't need a destructor, copy ctor or assignment op.

• get rid of the "inline" keywords - they are doing nothing.

Answer 2

When you call it with out the reference, its copying units and *name but not the values inside of *name. So when that temporary object is destructed, its freeing *name from all instances of Configuration.

Answer 3

you have to add your struct a copy constructor and handle char *name; (mean, allocate and delete memory, initialize with value).

Anyway, it is a not good idea to use char * for strings. Use std::string, that will handle everything for you.

Answer 4

You didn't provide your own copy constructor or assignment operator. So, when you make a copy or an assignment, the compiler-generated copy constructors and assignment operators are used which actually don't do the right thing in this case. They simply copy every member so you end up with two Configuration objects referring to the same character array. And both Configuration objects feel responsible for deleting the array which almost certainly leads to a "double-deletion" error.

Keep in mind the "rule of three". The problem here is that the pointer doesn't behave like you want it to. If you had used a std::string as member you wouldn't have to write your own copy constructor, destructor, assignment operator. That's because the compiler-generated ones simply invoke the relevant operations on their members and the string-member already handles this correctly -- unlike a pointer to char.