EXC_BAD_ACCESS during Copy Constructor

[Philip Lee Bridson]

Good Evening,

I have been writing apps in C++ for a while now but I have to admit
the below problem has stumped me - I would not consider myself an
'expert' but also I am not a novice. Any help would be appreciated.

I have a copy constructor which is defined as follows:

Object::Object(const Object& o) {

if (o != NULL) {
if (_internalObject != NULL) {
delete _internalObject;
_internalObject = NULL;
}

_message = ex.Message();

if (o.InternalObject() != NULL) {
_internalObject = new Object();
_internalObject = ex.InternalObject();
}
}
}

When I get to the line that deletes the internal object (which is a
pointer that is initialized during the constructor) I get
EXC_BAD_ACCESS. I have debugged the app and I can confirm that I am
not calling a double free() and that _internalObject actually does
piont to something.

The code from which the copy constructor is initiated is below...

Object * o = new Object();

if (o != NULL) {Object p = *o; //etc....}

the app only crashes when I copy the object, all other constructors
are fine. Does anyone have any ideas what may cause this?

Many thanks in advance.

Phil.

 

[Öö Tiib]

Good Evening,

I have been writing apps in C++ for a while now but I have to admit
the below problem has stumped me - I would not consider myself an
'expert' but also I am not a novice. Any help would be appreciated.

I have a copy constructor which is defined as follows:

Object::Object(const Object& o)

Where is
: _intrnalObject( NULL )
???
Add it to initialize _internalObject.

{

        if (o != NULL) {

how can you compare o with NULL? Does it have operator int or
operator != (int)?

            if (_internalObject != NULL) {
                delete _internalObject;
                _internalObject = NULL;
            }

Your _internalObject is not initialized, so it possibly points at
garbage. Initialize it and remove above block.

            _message = ex.Message();

What is ex?

 

[Victor Bazarov]

Good Evening,

I have been writing apps in C++ for a while now but I have to admit
the below problem has stumped me - I would not consider myself an
'expert' but also I am not a novice. Any help would be appreciated.

I have a copy constructor which is defined as follows:

Object::Object(const Object& o) {

So, this is a constructor, right? Any member have been initialized yet?
Probably not (you don't have any initializer list, do you?), so what
do you expect their values are? You compare '_internalObject', which
has some indeterminate value here, to NULL. Why would it be NULL except
by chance? Oh, it can be NULL if your 'Object' is static, yes. But you
don't usually copy-construct statics, do you?

if (o != NULL) {

Your 'Object' has a conversion to 'int'? Why would you compare 'o' to 0
here?

if (_internalObject != NULL) {
delete _internalObject;
_internalObject = NULL;
}

It seems you copied your *constructor* code from an assignment operator
or something like that. Well, don't. And if you do copy code from some
other place, you *should* examine it carefully to ensure that every line
of your code makes sense in the *new location of the code*.

_message = ex.Message();

if (o.InternalObject() != NULL) {
_internalObject = new Object();
_internalObject = ex.InternalObject();
}
}
}

When I get to the line that deletes the internal object (which is a
pointer that is initialized during the constructor)

It's NOT a pointer that is initialized to anything at the time you're
trying to delete it.

> I get
EXC_BAD_ACCESS. I have debugged the app and I can confirm that I am
not calling a double free() and that _internalObject actually does
piont to something.

What does it point to? Anything you have control over?

The code from which the copy constructor is initiated is below...

Object * o = new Object();

if (o != NULL) {Object p = *o; //etc....}

'o' is never NULL when 'new' returns it. 'new' throws if you don't have
enough memory (or if the constructor throws).

the app only crashes when I copy the object, all other constructors
are fine. Does anyone have any ideas what may cause this?

Yes, just like I told you above, since '_internalObject' is not
initialized to anything, it contains some random garbage that you make
the machine interpret as a pointer to something. The machine cannot
make sense of it, and you get notified.

This is what your code should look like:

Object::Object(const Object &other)
: _internalObject(other._internalObject ?
new Object(*other._internalObject) : 0)
{
}

And just for completeness' sake, read about "the Rule of Three" if you
haven't yet.

V

 

[Philip Lee Bridson]

So, this is a constructor, right?  Any member have been initialized yet?
  Probably not (you don't have any initializer list, do you?), so what
do you expect their values are?  You compare '_internalObject', which
has some indeterminate value here, to NULL.  Why would it be NULL except
by chance?  Oh, it can be NULL if your 'Object' is static, yes.  But you
don't usually copy-construct statics, do you?




Your 'Object' has a conversion to 'int'?  Why would you compare 'o' to 0
here?


It seems you copied your *constructor* code from an assignment operator
or something like that.  Well, don't.  And if you do copy code from some
other place, you *should* examine it carefully to ensure that every line
of your code makes sense in the *new location of the code*.






It's NOT a pointer that is initialized to anything at the time you're
trying to delete it.

 > I get


What does it point to?  Anything you have control over?




'o' is never NULL when 'new' returns it.  'new' throws if you don't have
enough memory (or if the constructor throws).


Yes, just like I told you above, since '_internalObject' is not
initialized to anything, it contains some random garbage that you make
the machine interpret as a pointer to something.  The machine cannot
make sense of it, and you get notified.

This is what your code should look like:

    Object::Object(const Object &other)
      : _internalObject(other._internalObject ?
                         new Object(*other._internalObject) : 0)
    {
    }

And just for completeness' sake, read about "the Rule of Three" if you
haven't yet.

V

Thanks all for the advice. Your posts were very helpful. I understand
why I was going wrong now and I will ammend the lines that are causing
the issue. Kind Regards, Phil.