Setting Pointer address to NULL

[JRimmer]

I'm trying to set a pointers address to NULL after I delete the information stored in it. It may be an easy solution, and I actually hope it is, but I don't see this solution.

Thanks for taking a look.

 #include "Geometry.h"


Geometry::Geometry(){
	name = new char[20];
	name = "Geometry Default N";
	type = new char[20];
	type = "Geometry Default T";
}

Geometry::Geometry( char *name, char *type){
	this->name = name;
	this->type = type;
}

Geometry::Geometry(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
}

Geometry & Geometry::operator=(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
}

Geometry::~Geometry(){
        //Here is where I'm running into the issue.
	delete [] name;
	name = NULL;
	delete [] type;
	type = NULL;
}

char Geometry::getName(){
	return *name;
}

char Geometry::getType(){
	return *type;
}

[Peter87]

The problem is in the constructor.

You allocate an array of 20 char and make name point to it (name points to the first char in the array to be precise).

name = new char[20];

Then you make name point to a string literal instead. Now you have no way of getting to the array that you created on the previous line.

name = "Geometry Default N";

In the destructor you use delete. delete should only be used on things that you created with new. name points to a string literal, which was not created with new so this is a problem.

delete [] name;

If name is a member of Geometry it's a bit unnecessary to set it to null in the destructor because it will no longer exist after the destructor has ended anyway.
name = NULL;



I guess what you really wanted to do on line 6 and 8 was to set the content of the arrays that you have allocated with new. In that case you can use std::strcpy.

std::strcpy(name, "Geometry Default N");

[Catfish666]

Your problems come from here:

Geometry::Geometry(){
	name = new char[20];
	name = "Geometry Default N";
	type = new char[20];
	type = "Geometry Default T";
}

What does the operator new[] do?
In fact, what is a pointer?

A pointer is a variable that stores a memory address.
A pointer is a bit like an unsigned long int, but the number it stores represents a memory address.

So what does new[] do? As in:

name = new char[20];

It allocates a chunk of memory and returns its memory address.
More specifically, in your case, it allocates memory for an array of 20 char's then returns the memory address of the first element of that array.

So far so good. But the problem is that you do:

	name = new char[20];
	name = "Geometry Default N";

The trick to understand is that "Geometry Default N" is a string literal. You can think of it as a nameless variable, and it too has a memory address.

So what happens is that you replace the memory address given by new[] with the memory address of the string literal, and the string literal cannot be delete[]'d.

Long story short: you cannot copy char arrays by simple assignment, which is why in good old C the strcpy() function exists, and in C++ there exists std::string as a replacement of char arrays and pointers.

So do what the C++ programmer does, and use std::string.

#include <string>

class Geometry
{
private:

    std::string name;
    std::string type;

// ...
};

#include "Geometry.h"

Geometry::Geometry(){
	// now this simply works
	name = "Geometry Default N";
	type = "Geometry Default T";
}

// ...

Geometry::~Geometry(){
	// nothing needs to be done with regards to
	// cleaning up this->name and this->type
}

[JRimmer]

Thanks you two for your detailed explanations!

I don’t know if this will work either, but I think this is closer to what I was trying to achieve.

*name = “Geometry Default N”;

What this code says to me is, store ”Geometry Default N” into the container pointed to by name. Please correct me if I’m wrong.

“If name is a member of Geometry it's a bit unnecessary to set it to null in the destructor because it will no longer exist after the destructor has ended anyway.” (Peter87)

Will this statement remain true if I only plan on using Geometry as an abstract class?

“The trick to understand is that ”Geometry Default” is a string literal. You can think of it as a nameless variable, and it too has a memory address.” (Catfish666)

This would lead me to believe that my statement made at the top of this post won’t work. Is this the case?

Also I would have loved to use strings, but for the purposes of this assignment my instructor has made it clear we are to use char pointers.

What is happening in the code below exactly? Does it mean name has the address of the string literal?

Geometry::Geometry(){
	strcpy(name, "Geometry Default N");
	strcpy(type, "Geometry Default T");
}

One final question, won’t I need to delete name; and delete type; and set each of these pointers address to NULL (0) so there isn’t a risk of them being used again by accident after they have served their purpose?

[Catfish666]

No, dereferencing the name pointer doesn't work.
It's been explained that in C++, arrays can't be assigned.

*name = "Geometry Default N"; // is actually the same as
name[0] = "Geometry Default N"; // which won't work 

JRimmer wrote:

What is happening in the code below exactly? Does it mean name has the address of the string literal?

Geometry::Geometry(){
	strcpy(name, "Geometry Default N");
	strcpy(type, "Geometry Default T");
}

No. It means that the string literals will be copied onto the memory chunks that name and type are pointing to.

http://www.cplusplus.com/reference/cstring/strcpy/

Here is a simplified implementation of std::strcpy() to illustrate how the "real" one works:

#include <cstddef>
#include <iostream>

void example_strcpy(char *dest, const char *src)
{
    std::size_t i=0;

    do
    {
        dest[i] = src[i];
        std::clog << "dest[" << i << "] := '";

        if (src[i] == '\0')
            std::clog << "\\0' (NUL character)";
        else
            std::clog << src[i] << '\'';

        std::clog << '\n';
    }
    while (src[i++] != '\0');
}

int main()
{
    char name[40];

    example_strcpy(name, "Wolverine");
    std::cout << "name is: " << name << std::endl;
}

Test run of the above:
http://ideone.com/9E3bGa

You may want to also read this:
http://www.cplusplus.com/faq/sequences/strings/c-strings-and-pointers/

JRimmer wrote:

One final question, won’t I need to delete name; and delete type; and set each of these pointers address to NULL (0) so there isn’t a risk of them being used again by accident after they have served their purpose?

What is this? Defensive programming against... yourself? Or against other programmers?

Of course there must exist cases when clearing a pointer to NULL (or clearing data in general) makes sense, so as to prevent malicious users from causing problems somehow.

However in your case, as Peter87 already pointed out, the destructor is the last place where you will use those pointers. So you won't get a chance to misuse them after they've been delete[]'d.

[JRimmer]

Thanks for taking the time to reply.

I have understood some of what you have said, some of it is still above my head. I'll read over it again and explore it a little more closely later, but due to time constraints I was hoping you could answer this final question.

Given the following class:

#ifndef GEOMETRY_H
#define GEOMETRY_H

class Geometry
{
	protected: 
		char *name, *type;
	public:
		Geometry();
		Geometry( char *, char *);
		Geometry(const Geometry &);
		Geometry & operator=(const Geometry &);
		//~Geometry();

		char getName();
		char getType();
		virtual double ComputeVolume()=0;
		virtual double ComputeSurface()=0;
};

#endif 

What code would you write inside the default constructor so you could assign the two string literals to each char pointer? I'm getting a runtime error, and when I've run the program in debugger mode, I noticed my string literals were not being pointed to by the char pointers. Specifically I get he following error for the value of *char and *type = 0xccccccc <Error reading characters of string.>.

#include "Geometry.h"
#include <string>


Geometry::Geometry(){
	strcpy_s(name, 19, "Geometry Default N");
	strcpy_s(type, 19, "Geometry Default T");
}

Geometry::Geometry( char *name, char *type){
	this->name = name;
	this->type = type;
}

Geometry::Geometry(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
}

Geometry & Geometry::operator=(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
	return *this;
}
/*
Geometry::~Geometry(){
	delete name;
	name = NULL;
	delete type;
	type = NULL;
}
*/
char Geometry::getName(){
	return *name;
}

char Geometry::getType(){
	return *type;
}

[JRimmer]

I figured it out:

Geometry.cpp

#include "Geometry.h"
#include <iostream>
#include <string>


Geometry::Geometry(){
	name = new char[30];
	type = new char[30];
	strcpy_s(name, 30, "Geometry Default N");
	strcpy_s(type, 30, "Geometry Default T");
}

Geometry::Geometry( char *name, char *type){
	this->name = new char[strlen(name)+1];
	this->type = new char[strlen(type)+1];
	strcpy_s(this->name,strlen(name)+1,name);
	strcpy_s(this->type,strlen(type)+1,type);
}

Geometry::Geometry(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
}

Geometry & Geometry::operator=(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
	return *this;
}

Geometry::~Geometry(){
	delete  name;
	delete  type;
}

char* Geometry::getName(){
	return name;
}

char* Geometry::getType(){
	return type;
}

Thanks for the help!

[Catfish666]

Looking good but there is one small mistake: mismatched new[]/delete pair.

new must be matched with delete.
new[] must be matched with delete[].

Example:

int *pi1 = new int; // have used new
delete pi1; // so using delete

int *pi2 = new int[10]; // have used new[]
delete[] pi2; // so using delete[] 

So your Geometry destructor should be corrected to:

Geometry::~Geometry(){
	delete[] name;
	delete[] type;
}

(I hope Cubbi won't see this link.)
http://yosefk.com/c++fqa/heap.html#fqa-16.12

Edit: wait, not looking good.
Your copy constructor and copy assignment operator are problematic:

Geometry::Geometry(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
}

Geometry & Geometry::operator=(const Geometry & a){
	this-> name = a.name;
	this-> type = a.type;
	return *this;
}

First of all the small stuff: in the copy assignment operator you must check for self-assignment:

Geometry & Geometry::operator=(const Geometry & a){
	if (this != &a)
	{
		// ...
	}
	return *this;
}

http://www.parashift.com/c++-faq/assignment-operators.html

Otherwise you may have problems when someone does something like:

Geometry g;

g = g; // this is very obviously a mistake in this example,
// but it can happen in less obvious ways (for example by using references)

Geometry &rg = g; // reference to g (same object, different name)
g = rg; // same self-assignment problem! 

Now for the bigger problem in your code: the copy constructor and copy assignment operator effectively share the resources with the a object, instead of copying the resources as their own.

This means that when name and type are modified in one object, they will be modified in the other object as well.

Even worse, when the objects will be destructed, their destructors will each delete[] the same name and type pointers, which means they will be delete[]'d twice, which is a no-no.

http://yosefk.com/c++fqa/heap.html#fqa-16.2

So finally your code will look a bit like this:

Geometry::Geometry(const Geometry & a){
    this->name = new char[strlen(a.name) + 1];
    this->type = new char[strlen(a.type) + 1];
    strcpy_s(this->name, strlen(a.name) + 1, a.name);
    strcpy_s(this->type, strlen(a.type) + 1, a.type);
}

Geometry & Geometry::operator=(const Geometry & a){
    if (this != &a)
    {
        delete[] this->name;
        delete[] this->type;
        this->name = new char[strlen(a.name) + 1];
        this->type = new char[strlen(a.type) + 1];
        strcpy_s(this->name, strlen(a.name) + 1, a.name);
        strcpy_s(this->type, strlen(a.type) + 1, a.type);
    }
    return *this;
}

[JLBorges]

> http://yosefk.com/c++fqa/heap.html#fqa-16.12

This yosefk does not know much about memory allocation, does he?

The bottom line is this: C++ stores the number of objects in a block once when new is called and twice when new[] is called.

Doesn't he know that operator new may be overloaded to allocate from a pool allocator (size not stored with each block), while operator new[] may be using a block allocator (size stored with each block)? That the overall C++ philosophy is 'do not pay for what you do not use' - that if we are not using dynamically allocated arrays of objects, we shouldn't have to incur an extra cost of managing object arrays for every single object that we create?

EDIT: Deep copy / assignment part deleted. (Catfish666 edit covers it).