Archived

This topic is now archived and is closed to further replies.

MagTDK

Pointer to pointer

Recommended Posts

I'm having a problem with pointers to pointers. I have several functions like so:
    
void SomeFunction();
{
byte *pData;
ProcessData (&pData);
// code that's not important for this question.

delete [] pData; pData = NULL; // delete pointer that was created in the convertdata function.

}

void ProcessData (byte **pData)
{
// code that's not important for this question

ConvertData (pData);
}


void ConvertData (byte **pData)
{

// code that's not important for this question

*pData = new byte[1000] // allocate memory

// code that fills what pData is pointing to

}
    
Problem is when I run the program I get an unhandled exception error. If I combine the code in the convertdata function with the processdata function, therefor eliminating the convertdata function, the program runs with no error. So I take it my problem has to do with passing the pointer to the 2nd function (convertdata). Note: I'd like to keep these two functions seperate as each function is quite large. So my question is how do I pass a pointer through two functions rather than one? [edited by - MagTDK on August 12, 2002 5:30:46 AM]

Share this post


Link to post
Share on other sites
I think it''s

*pData = new byte[1000];

You''ll have to change it to

*pData = new byte*[1000];

Oh and why do you do this

byte *pData;
ProcessData (&pData);

This may cause problems try this

byte **pData;
ProcessData (pData);

Share this post


Link to post
Share on other sites
Your code looks fine to me, although I can''t see why you''d want to pass a pointer to a pointer (instead of just a pointer). The error is probably in the code you omitted. Have you actually checked which line the exception occurs?

____________________________________________________________
www.elf-stone.com

Share this post


Link to post
Share on other sites
Yeah, it occurs when I try to delete the pointer:
delete [] pData; pData = NULL;

I checked the address in the debugger and it appears to be correct.

I just can't see why it would work if I copy everything from the convertdata function into the processdata when it's basically the same thing except for the call to the convertdata function.

quote:

Your code looks fine to me, although I can't see why you'd want to pass a pointer to a pointer (instead of just a pointer).



Don't I need the pointer to pointer when allocating memory for a pointer that was created outside of this function?

[edited by - MagTDK on August 12, 2002 6:20:17 AM]

Share this post


Link to post
Share on other sites
Change you function definitions to:

  
void ProcessData (byte *&pData)
void ConvertData (byte *&pData)



and things shoud be okay.

[edited by - carrot on August 12, 2002 6:33:01 AM]

Share this post


Link to post
Share on other sites
quote:
Don't I need the pointer to pointer when allocating memory for a pointer that was created outside of this function?

no.

It sounds like you've deleted the pointer when it hasn't been allocated. The access violation probably occurs because you try to delete an invalid pointer.

You should set your pointer to NULL when you initialise it so that you can tell if it's valid later on. You can then check the validity of the pointer before you attempt to delete it.

ie:

byte *pData=NULL;
ProcessData (&pData);// code that's not important for this question.
if (pData!=NULL)
{
delete [] pData;
pData = NULL;
}




____________________________________________________________
www.elf-stone.com

[edited by - benjamin bunny on August 12, 2002 6:44:05 AM]

Share this post


Link to post
Share on other sites
void SomeFunction();
{
byte *pData;
ProcessData (&pData);
// code that's not important for this question.
delete [] pData;

This line is redundant:
pData = NULL; // delete pointer that was created in the convertdata function.
}

There's nothing technically wrong with this, but you should prefer to pass by reference here. Make the signature

void ProcessData(byte *&pData)

void ProcessData (byte **pData)
{
// code that's not important for this question

Whatever you've omitted here might be relevant. You can get a problem with deleting data when you have trashed the heap somewhere along the way.

ConvertData (pData);
}

Again, prefer pass-by-reference here:
void ConvertData (byte **pData)
{

// code that's not important for this question
*pData = new byte[1000] // allocate memory
// code that fills what pData is pointing to

Another location where you might be trashing the heap.
}

quote:

So I take it my problem has to do with passing the pointer to the 2nd function (convertdata).

You can confirm that in your debugger.

[edited by - SabreMan on August 12, 2002 6:55:43 AM]

Share this post


Link to post
Share on other sites
quote:
Original post by benjamin bunny
It sounds like you''ve deleted the pointer when it hasn''t been allocated. The access violation probably occurs because you try to delete an invalid pointer.



Correct.

quote:

You should set your pointer to NULL when you initialise it so that you can tell if it''s valid later on. You can then check the validity of the pointer before you attempt to delete it.

ie:

byte *pData=NULL;
ProcessData (&pData);// code that''s not important for this question.
if (pData!=NULL)
{
delete [] pData;
pData = NULL;
}




Wrong. You can delete null pointers. You set pointers to null to make sure that delete knows not to, well, delete it. The following code will work just fine:


int main()
{
int *ptr = 0;
delete ptr;
return 0;
}


It''s when you try to delete an invalid pointer that is when you get problems. Null pointers are perfectly valid.

Death of one is a tragedy, death of a million is just a statistic.

Share this post


Link to post
Share on other sites
quote:

You should set your pointer to NULL when you initialise it so that you can tell if it's valid later on. You can then check the validity of the pointer before you attempt to delete it.

ie:

byte *pData=NULL;
ProcessData (&pData);// code that's not important for this question.
if (pData!=NULL)
{
delete [] pData;
pData = NULL;
}




quote:

Wrong. You can delete null pointers. You set pointers to null to make sure that delete knows not to, well, delete it. The following code will work just fine:



I stand corrected. I hadn't noticed that delete checks for null pointers because I've got into the habit of always checking pointers are valid before I do anything with them.

But anyway, the point [edit: that's not supposed to be a pun] is that he should set the pointer to NULL when he initialises it so that he can check its validity before he uses it.

____________________________________________________________
www.elf-stone.com

[edited by - benjamin bunny on August 12, 2002 7:22:10 AM]

Share this post


Link to post
Share on other sites
quote:
Original post by benjamin bunny
But anyway, the point [edit: that''s not supposed to be a pun] is that he should set the pointer to NULL when he initialises it so that he can check its validity before he uses it.

A better solution would be to encapsulate the pointer and all manipulation of it within a handle class.

Share this post


Link to post
Share on other sites
quote:
Original post by benjamin bunny
I''m not sure that "why bother" constitutes a tantrum.

Ah, I interpreted it as "why bother contributing to the thread", but I now see you meant "why bother using a handle class". My apologies...
quote:

I was simply making the point that your method was somewhat unnecessary.

That depends on what you mean by "necessary". I perceive it as necessary to do so when tackling such problems in my own (and my employers'') code for several reasons:

- it encapsulates memory management which helps with fault detection and enables transparency of change;

- it raises the level of intentionality, since user code can be seen to be acting at the intended abstraction level rather than dealing with pointer tracking and memory management;

- user code is vastly simplified, which means it is quicker and easier to get things right.

Usefully for the purposes of this post, there is an article on this very topic in this month''s CUJ, written by Koenig and Moo. It doesn''t appear to be on-line, so here''s a quote:

quote:

Class authors who wish to avoid managing memory can foist off the problem on their users. Indeed, doing so may be tempting, because ignoring memory management makes classes easier to write. However, such classes are harder for users to use, and the overall complexity of a system increases greatly if user code must manage the system''s memory directly.


It may seem a lot of initial investment to come up with a correctly working handle class, but IME that investment is usually more than repaid, and it soon becomes second nature to work in that way. I''m fairly convinced that the OP could avoid writing his own handle class if he refactored his design, and could potentially make use of the Boost libraries. I''d say a significant percentage of problems posted to these forums could be avoided with a simple change of mindset.

Share this post


Link to post
Share on other sites

Wow, I didn''t realize this thread was still going

quote:

So many guys have been trying to help u man, u say u found the error and u don''t wanna tell



When benjamin said that things looked ok, I went back and looked at my code again (I guess I just needed someone to confirm that things looked ok) Anyways, the error was really a dumb one...when I copied my data from one function over to the other I was missing two lines of code. I somehow manage to do that twice when copying and pasting. That''s all it took to create that error.

Originally I''d put the data into a class and reference it, but I found it was more trouble then what it was worth for this particular situation because I''m dealing with image data that has to be in contiguous order for D3D to load it. I would have had to create another function to swap the data in contiguous order because the class I had contained the header info and a pointer to the image data. As you can see, the pointer would have been the problem. This wasn''t a big deal, it just made things more messy.

Also I do set my pointers to NULL, but for some reason I didn''t their It''s probably because I just wrote the code.

Anyways thanks everyone for your help.

Share this post


Link to post
Share on other sites
SabreMan:
Well I take the point that it can simplify things to have memory management handled for you. I've done similar things to manage particles and physics objects in my engine. But, for the purpose of an array which is only needed for the lifetime of a function, IMO it's better to manage the memory yourself. I suppose it's a matter of preference as much as anything.

____________________________________________________________
www.elf-stone.com

[edited by - benjamin bunny on August 12, 2002 9:35:28 AM]

Share this post


Link to post
Share on other sites
quote:
Original post by benjamin bunny
But, for the purpose of an array which is only needed for the lifetime of a function, IMO it''s better to manage the memory yourself.

It''s not completely clear what the OP requires as I don''t know the full usage context, but the scenario you describe here implies a vector, the rationale of which is exlained in Cline''s Arrays are Evil.
quote:

I suppose it''s a matter of preference as much as anything.

I actually think there''s a fairly convincing argument that lifts certain techniques above mere preference, but I really can''t be arsed making that argument. It''s to be found in most reputable C++ publications.

Share this post


Link to post
Share on other sites