views:

66

answers:

3

Hi,

I have the following scenario.

class foo
{
  ...
  private:
   char *_test;
};


void foo::setTest()
{
  if( 0 != _test ) delete [] _test;
}

The function setTest throws an error when first run as it trying to delete _test when it has not been assigned yet. This is happening because _test is not set to 0X0.

Can anyone help me to understand this?

Thanks in advance.

+1  A: 

Well, if _test has not been assigned any value, then it has undefined value. You can't use that value for anything, if you want sane behaviour. Variables need to be assigned before being used.

Incidentally, delete and delete[] is safe to call on null pointers, so the == 0 check is redundant.

Chris Jester-Young
+1 For don't check for null.
GMan
+3  A: 

You should initialize _test with NULL in the constructor.

Like:

foo:foo {
 _test = NULL;
}

If you don't _test will have garbage value.

Also as pointed by Chris, The value passed as argument to delete or (delete[]) must be either a pointer to a memory block previously allocated with new, or a null pointer (in the case of a null pointer, delete produces no effect), effectively making your NULL check redundant.

codaddict
As the question says, `_test` isn't being initialised at all. :-P
Chris Jester-Young
the correct syntax would be: `foo::foo() : _test(0) {}`, where the `0` is optional (but more explicit to read) as `_test()` will value initialize that for a pointer will mean 'set to 0' anyway.
David Rodríguez - dribeas
+1  A: 

There are two possibilities:

If you never initialize foo::_test, then that variable will likely contain random data when your class is constructed. C++ does not initialize your pointers to null (unlike Java, C#, or most other higher-level languages). You should always (ALWAYS!) initialize your pointers to NULL or a valid value. Change to char *_test = NULL; or otherwise initialize _test in your constructor.

Alternately, it's possible that _test is being deleted in a different place not shown in your sample, but isn't being set to NULL. Whenever you delete a class member, you should set it to NULL afterwards in order to prevent this sort of double-free problem.

JSBangs
In C++, the null pointer is either called 0 or `nullptr` (the latter only in C++0x). It's not called `null`.
Chris Jester-Young
Could be also that the object is being copied and there's no copy constructor.
asveikau
@Chris Jester-Young: If you are going to nit pick, make sure you know the details. In C++03 (current standard) the pointer is in fact called 'null' everywhere in the standard where it is defined as the pointer obtained from converting the integer value `0` to a pointer type. The standard also requires `NULL` to be a macro that is defined to `0` (unlike C, where it is defined as a pointer type in the standard, usually `(void*)0` in implementations). The upcoming standard will define `nullptr` to be a null pointer value.
David Rodríguez - dribeas
@David: The original answer said `null` (in `<code>` blocks), implying that you have `null` literally in program text. That was what I was correcting. The answer's author amended the `null` to become `NULL`, which of course is fine (I just didn't bother to comment further on it).
Chris Jester-Young
@David: Personally, I detest the use of `NULL` (partly because, as a macro, it's "second-class" compared to 0 or `nullptr`), so I didn't mention it in my list of choices in the initial comment (and why I didn't comment further on it when the answer was changed to use it).
Chris Jester-Young