Sign in to follow this  
Zdman

Deleting triple pointer, double pointers it's just not working

Recommended Posts

i don't know if anyone will be able to help but i keep getting a debug error of DAMAGE: after Normal block (#71) at 0x00491E20. If I press ignore it will continue. When I get to delete [] models[i], I get a Debug assertion Failed...Expresion: _CrtIsValidHeapPointer(pUserData)----after this i get an unhandled expression and it crashes. I don't know what is causing this. I think I'm doing it right. Here's that section of code. char buffer[255]; char nLoop; int num_dealers; char** dealers; char*** models; int* num_of_models_per_dealer; double** prices; double* averages; .......... for(int i = 0; i < num_dealers; i++) { delete [] dealers[i]; }; delete [] dealers; for(i=0; i < num_dealers; i++) { for(int j=0; j < num_of_models_per_dealer[i]; i++) { delete [] models[i][j]; }; delete [] models[i]; }; delete [] models; for(i=0; i < num_dealers; i++) { delete [] prices[i]; }; delete [] prices; delete [] averages; delete [] num_of_models_per_dealer;

Share this post


Link to post
Share on other sites
better? [smile]
i believe for the delete [] dealers[i]
you can just do delete dealers[i], unless dealers[i] points to other pointers.
either that or your syntax is wrong....

char buffer[255];
char nLoop;
int num_dealers;
char** dealers;
char*** models;
int* num_of_models_per_dealer;
double** prices;
double* averages;

..........

for(int i = 0; i < num_dealers; i++)
{
delete [] dealers[i];
}
delete [] dealers;
for(i=0; i < num_dealers; i++)
{
for(int j=0; j < num_of_models_per_dealer[i]; i++)
{
delete [] models[i][j];
};
delete [] models[i];
}
delete [] models;
for(i=0; i < num_dealers; i++)
{
delete [] prices[i];
}
delete [] prices;
delete [] averages;
delete [] num_of_models_per_dealer;

Share this post


Link to post
Share on other sites
I would suggest setting your pointers to NULL after clearing them. This way, if you accidently use or delete them afterwards, you will get a much more trackable crash.

Another thing I have found useful in the past is to do this:

Dealer* pDealer = dealers[i];
delete pDealer;
dealers[i] = NULL;

instead of:

delete dealers[i];

Not a big deal, but it helps me make sure I know what I am deleting. It also helps when code changes; if the type is explicit, it helps remind me that I need to change that type. Delete will just deallocate whatever at that address, regardless of the object type.

You can try scattering _CrtCheckMemory() calls around to track down the source. Take a look at the compiler documentation for this function. This may trap the problem earlier and help you find out which deletion is causing issues.

A few other items to consider:
Are you sure all of these pointers are assigned to valid objects?
Are you sure all of your array indices are in bounds?
Are you sure all of these objects are allocated on the stack?
Have you walked through it in the debugger to make sure all of your assumptions are correct?

Share this post


Link to post
Share on other sites
That's a neat idea with allocating a pointer. But I don't think it'll help too much. I have stepped through it and have tried some of the things above. To me it seem like it has deleted if if I press ignore and everyone of them do it so it's something in all of them. They all hold valid objects because I've used them in the body of the program and it all works. The pointers you see are on the stack but everything they contain are created are not. I don't know what _CrtCheckMemory() is so if you could explain. My code isn't very long so I'm going to post it here.

#include <iostream>
#include <iomanip>
#include <string>
#include <math.h>
#include <stdio.h>
#include <ctype.h>

using namespace std;

//Constants

//Function Declarations
int readNumDealers( );
//Reads the number of dealers from the user. You may call the function atoi( ) provided in the standard library within this function. You will have to include stdlib.h to use this function. The function atoi( ) converts a passed string to an integer and returns the integer, or 0 if the conversion cannot be made. The function prototype for atoi( ) is: int atoi( const char *nPtr).

char** readDealers( int num_of_dealers);
//Reads the names of the dealers from the user. Returns the names via a double pointer.
//Use new memory to allocate an array of pointers, each of which points to a dealer's name. Use the num_of_dealers parameter to specify the size of the array of pointers. Allocate an array of characters for each dealer and store that pointer in the array of pointers.

char*** readModels(int num_of_dealers, char**dealer_names, int *num_of_models_per_dealer);
//Reads the names of each model sold for a specified dealer. This function will need to prompt for the number of models sold for a specific
//dealer. This number of models will be maintained in the num_of_models_per_dealer array. Use the dealer_names to make your prompt
//more user friendly


double **readPrices( int no_of_dealers, char **dealers, char ***models, int *num_of_models_per_dealer);
//This function allows the user to enter the price for each model (pertaining to a specific dealership) and stores the price of that model in the
//appropriate location in the dynamically allocated double** described above. You will need to dynamically allocate the double** in this
//function. Use the dealers and models arrays to make your prompt more user friendly:


double *calcAvg(int no_of_dealers, double **prices, int *num_of_models_per_dealer);
//This function calculates the average price of all models that a dealer sells. The average price is stored in the average array described above.



void displayInfo(int no_of_dealers, int *num_of_models_per_dealer, char **dealers, char ***models, double** prices, double *avg);
//Displays all information pertaining to dealerships and their models . Example Output:


//Functions
int main()
{
char buffer[255];
char nLoop;
int num_dealers;
char** dealers;
char*** models;
int* num_of_models_per_dealer;
double** prices;
double* averages;

do{
num_dealers = readNumDealers();
dealers = readDealers(num_dealers);
num_of_models_per_dealer = new int[num_dealers];
models = readModels(num_dealers, dealers, num_of_models_per_dealer);
prices = readPrices(num_dealers, dealers, models, num_of_models_per_dealer);
averages = calcAvg(num_dealers, prices, num_of_models_per_dealer);
displayInfo(num_dealers, num_of_models_per_dealer, dealers, models, prices, averages);

for(int i = 0; i < num_dealers; i++)
{
delete [] dealers[i];
};
delete [] dealers;
for(i = 0; i < num_dealers; i++)
{
for(int j = 0; j < num_of_models_per_dealer[i]; i++)
{
delete []models[i][j];
};
delete [] models[i];
};
delete [] models;
for(i = 0; i < num_dealers; i++)
{
delete [] prices[i];
};
delete [] prices;
delete [] averages;
delete [] num_of_models_per_dealer;
//Loop
cout << "Would you like to enter more Dealerships?(Y or N) ";
cin.getline(buffer, sizeof(buffer));
nLoop = buffer[0];
}while(nLoop == 'Y' || nLoop == 'y');

return 0;
}


//Reads the number of dealers from the user. You may call the function atoi( ) provided in the standard library within this function. You will have to include stdlib.h to use this function. The function atoi( ) converts a passed string to an integer and returns the integer, or 0 if the conversion cannot be made. The function prototype for atoi( ) is: int atoi( const char *nPtr).
int readNumDealers( )
{
char buffer[255];

cout << "How many dealers are there? ";
cin.getline(buffer, sizeof(buffer));

return(atoi(buffer));
}

//Reads the names of the dealers from the user. Returns the names via a double pointer.
//Use new memory to allocate an array of pointers, each of which points to a dealer's name. Use the num_of_dealers parameter to specify the size of the array of pointers. Allocate an array of characters for each dealer and store that pointer in the array of pointers.
char** readDealers(int num_of_dealers)
{
char buffer[255];
char** dealers;

dealers = new char*[num_of_dealers];

for(int i = 0; i < num_of_dealers; i++)
{
cout << "Please enter the name of dealer #" << i + 1 << ": ";
cin.getline(buffer, sizeof(buffer));
dealers[i] = new char[strlen(buffer)];
strcpy(dealers[i], buffer);
};

return dealers;
}

//Reads the names of each model sold for a specified dealer. This function will need to prompt for the number of models sold for a specific
//dealer. This number of models will be maintained in the num_of_models_per_dealer array. Use the dealer_names to make your prompt
//more user friendly
char*** readModels(int num_of_dealers, char**dealer_names, int *num_of_models_per_dealer)
{
char buffer[255];
char*** models;

models = new char**[num_of_dealers];

for(int i = 0; i < num_of_dealers; i++)
{
cout << "Please enter the number of models for " << dealer_names[i] << ": ";
cin.getline(buffer, sizeof(buffer));
num_of_models_per_dealer[i] = atoi(buffer);
models[i] = new char*[num_of_models_per_dealer[i]];
for(int j = 0; j < num_of_models_per_dealer[i]; j++)
{
cout << " Please enter the name of model #" << j + 1 << ": ";
cin.getline(buffer, sizeof(buffer));
models[i][j] = new char[strlen(buffer)];
strcpy(models[i][j], buffer);
};
};

return models;
}

//This function allows the user to enter the price for each model (pertaining to a specific dealership) and stores the price of that model in the
//appropriate location in the dynamically allocated double** described above. You will need to dynamically allocate the double** in this
//function. Use the dealers and models arrays to make your prompt more user friendly:
double **readPrices(int no_of_dealers, char **dealers, char ***models, int *num_of_models_per_dealer)
{
char buffer[255];
double** prices;

prices = new double*[no_of_dealers];

for(int i = 0; i < no_of_dealers; i++)
{
prices[i] = new double[num_of_models_per_dealer[i]];
for(int j = 0; j < num_of_models_per_dealer[i]; j++)
{
cout << "Please enter the price for " << dealers[i] << "'s " << models[i][j] << ": ";
cin.getline(buffer, sizeof(buffer));
prices[i][j] = atoi(buffer);
};
cout << endl;
};

return prices;
}

//This function calculates the average price of all models that a dealer sells. The average price is stored in the average array described above.
double *calcAvg(int no_of_dealers, double **prices, int *num_of_models_per_dealer)
{
double* averages;

averages = new double[no_of_dealers];

for(int i = 0; i < no_of_dealers; i++)
{
averages[i] = 0;
for(int j = 0; j < num_of_models_per_dealer[i]; j++)
{
averages[i] += prices[i][j];
};
averages[i] /= num_of_models_per_dealer[i];
};

return averages;
}

//Displays all information pertaining to dealerships and their models.
void displayInfo(int no_of_dealers, int *num_of_models_per_dealer, char **dealers, char ***models, double** prices, double *avg)
{
cout << setprecision(2) << fixed;
for(int i = 0; i < no_of_dealers; i++)
{
cout << "Information on dealership #" << i + 1 << endl;
cout << " Dealership name: " << dealers[i] << endl;
for(int j = 0; j < num_of_models_per_dealer[i]; j++)
{
cout << " " << models[i][j] << ": " << prices[i][j] << endl;
};
cout << "........................" << endl;
cout << " Average price of all models is " << avg[i] << endl;
cout << endl;
}
}

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
goodness, a triple pointer? I rarely use a double pointer anymore. Don't get so messed up in this pointer jungle and put things in containers and structs. Not sure about your scenario, but it helps to contain things better.

struct modelStruct {
model *model_data;
char *name;
//etc, you would only do this if it does help to give metadata
}

even more helpful is containers, like STL's

std::vector models;

Anyway, you might already know all this, but you shouldn't be touching raw pointers so much unless it's a core part of the system, and it doesn't really look like it.

Share this post


Link to post
Share on other sites
A DAMAGE error is *always* due to overwriting an array bounds.
First off, put asserts in around every array access to ensure that you are not going past the end (or start) of the array.

If you ever find yourself using three *'s in a row (***) then there's a good chance you're doing something wrong.
Second, I highly recommend you learn how to use references and const-references.

Third, there is your copy-and-paste bug in main:
      for(int j = 0; j < num_of_models_per_dealer[i]; i++)
It should be j++.

Share this post


Link to post
Share on other sites
Guest Anonymous Poster
"First off, put asserts in around every array access to ensure that you are not going past the end (or start) of the array."

Actually, you'd be better off fixing your code. You shouldn't use error handling as a crutch to cover up badly written code.

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
"First off, put asserts in around every array access to ensure that you are not going past the end (or start) of the array."

Actually, you'd be better off fixing your code. You shouldn't use error handling as a crutch to cover up badly written code.


Since when are asserts a 'crutch'. They're there to tell you about the error WHEN IT HAPPENS, instead of relying on some generic error message that has next to nothing to do with the actual error.

Share this post


Link to post
Share on other sites
Seriously, consider using std::string with std::lists.

Also, everywhere, where you do new char[strlen(buffer)]; you need to include one char more for the terminating 0.


[little rant]How come people throw around std::couts and everything nice and then work with char*?? There should be a switch forbidding the use of char pointers[/little rant]

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
"First off, put asserts in around every array access to ensure that you are not going past the end (or start) of the array."

Actually, you'd be better off fixing your code. You shouldn't use error handling as a crutch to cover up badly written code.


I'm finding it very hard not to immediately chrip "Idiot" at that remark. Take no little heed however as I am an asshat. Asserts are meant as a DEBUGGING TOOL, as evidenced by the fact that they're completely compiled out in release mode/with DEBUG defined. They help THE PROGRAMMER catch the ERRORS so (s)he can FIX them.

Error REPORTING is the first step to locating bugs and fixing them. I'd prefer a error REPORT over a random crash caused 3 hours into the execution of a program due to some corrupted data from a bad pointer or the like in the beginning of the program that would've been caught by an assert. And by "prefer" I mean "I'd fix the bug by first adding enough error-detecting code (asserts) to find out where the problem(s) are."

Also, IMNSHO error HANDLING implys some grace - aka, attempting to possibly recover, or displaying a message meaningful to "the end user". That is, I wouldn't consider asserts "Error HANDLING", although I would consider them "Error REPORTING".

dumbass

    -Mike

Share this post


Link to post
Share on other sites
Quote:
Original post by Zdman
I don't know what is causing this.


What's causing this is that you're in over your head.

Quote:

int num_dealers;
char** dealers;
char*** models;
int* num_of_models_per_dealer;
double** prices;
double* averages;


If you're not capable of solving the problem yourself, you shouldn't be playing with these kinds of toys.

Instead:


// Let's give some structure to things, with, well, structures.
// This way we'll avoid confusing ourselves with multiple levels of indirection,
// especially since before, the number of *'s on the chars didn't match the
// other things at the same "level" (because you needed one more star to say
// "this is a string", which is an ugly hack in and of itself).

struct model {
std::string name; // what was in 'char *** models', per char **
double price;
}

struct dealer {
std::string name; // what was in 'char ** dealers', per char *
std::vector<model> models; // also keeps track of the num_of_models
double average; // I assume this is a cache of the average price?
}

std::vector<dealer> dealers;

// Guess what? You won't have to do *any* new/delete business now! All the
// dynamic allocations that occur are managed by the vectors and strings.

// Also, because of the structure setup, the size and order of 'averages' is
// automatically kept in sync with the names of the 'dealers'; similarly the
// names vs. prices of 'models', etc.

Share this post


Link to post
Share on other sites
I figured it out, when i allocated a string... I overwrote the boundaries of an array, fixed first problem and thx for catching that other error with j. It's for class and I'm at the top of generally. I thought I had done it correctly but I had made two simple errors and gotten weird error reports i haven't seen before. Thx guys.

Share this post


Link to post
Share on other sites
Even if you solved your problem, I am still baffled as to why you need triple pointers? I mean, the average computer is fast enough that you don't need to do insane stuff for speed anymore. Almost everyone in the C++ area is well versed with STL, since it has good documentation, has been error tested by millions of developers.

Share this post


Link to post
Share on other sites
Quote:
Original post by Anonymous Poster
"First off, put asserts in around every array access to ensure that you are not going past the end (or start) of the array."

Actually, you'd be better off fixing your code. You shouldn't use error handling as a crutch to cover up badly written code.
Yeah I should know not to post sensible answers, that were very helpful in solving more than one of his problems.
Next time perhaps I should post utter garbage like you just did!

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

Sign in to follow this