🔒 Locked

pointer assignment problem... (new question)

Started by the_moo Apr 12, 2006 at 6:02 AM 7 replies 1k views

Original Post

Apr 12, 2006 at 6:02 AM

Please note that this problem has been solved already. The new question/problem is further down - my next post hey all, its been a while since i posted here so firstly, hope you're all well [wink]! now my problem is very strange, i have a block of code that determines a filename and then opens that file using ifstream. the strange thing is that on running the code once it works - however, when i release the loaded object and then try to re-create it (and hence re-open the file), the program crashes. another strange thing is that the same block of code used in another function works perfectly... the block is:


std::string animfilename;
std::string::size_type position;

animfilename = filename;
position = animfilename.find(".");
animfilename.erase(position);
animfilename += ".ani";
std::ifstream animfile(animfilename.c_str());

//then i use .getline to read the file
while (std::getline(animfile,line)) //etc.

basically i have an animation file of the same name as the sprite's or mesh's data file. the problem is that i get a memory cannot be "read" error when i try to open the file for the second time. i've tried many many things in order to fix this but no luck. if anyone can help or has any questions that might produce answers, please let me know! thanks [Edited by - the_moo on April 12, 2006 7:14:53 PM]

jflanglois

Apr 12, 2006 at 6:18 AM

Well, first of all, you can simplify your code quite a bit:

std::string temp( filename, 0, filename.find_last_of( "." ) );std::ifstream fin( ( temp + ".ani" ).c_str() );while( std::getline( ... ) ) { ... }

As for your main problem, I think we'll need more code. What line does your program break to?

jfl.

Ravuya

298

Ravuya

Apr 12, 2006 at 9:39 AM

Are you closing your file after you use it?

the_moo

Author

the_moo

Apr 12, 2006 at 7:56 PM

ok, i fixed the file problem. i believe it was me not closing it at the right time... anyway now i've discovered that that wasn't the main thing causing my game to crash:
the problem is similar in that it only happens after i call FreeResources or remove the object from memory once. the strange thing is when i remove an object i delete it from memory so when a new object of that type is created it should be the only one and i would think nothing different should come because of the last one... but that doesn't appear to be happening.

i know i need to provide exact code details so here is a list of actions taken to arrive at the problem:
1. Load the object (among others which don't experience this problem)
2. Remove the object (either all at once or just by itself)
3. Attempt to load the object again (with or without others)
4. Program crash with "memory cannot be "read"" error (window pops up)

now for the code. first is the section that when removed causes the program to run without crashing, but vital assignments are then missing (lines 1 and 2 must be removed for it to run):

pmenu = NULL;pmenu = new SubMenu;pmenu->parent = menulist[index];pmenu->name = ReadLineTo(pos,line,';');pmenu->submenus = NULL;pmenu->items = NULL;pmenu->nextmenu = NULL;pmenu->lastmenu = NULL;++menulist[index]->maxsel;              if (!menulist[index]->submenus)  menulist[index]->submenus = pmenu;           //1else{  pmenu->nextmenu = menulist[index]->submenus;   menulist[index]->submenus = pmenu;           //2  menulist[index]->submenus->nextmenu->lastmenu =     menulist[index]->submenus;}

ok, this basically creates a new submenu object for my menu systems. all variable assignments in the first block are fine - no problems ever. it's lines 1 and 2 that cause a crash. for some reason when i try to assign "pmenu" to any other pointer (so copy the pointer), it crashes - but only on the second time round...

so now i'll show you the remove code that first removes the menu from a list then releases all data:

MenuX *check = NULL;  check = FindMenu(menuname);if (check){  if (check->lastmenux)    if (check->nextmenux)    {      check->lastmenux->nextmenux = check->nextmenux;      check->nextmenux->lastmenux = check->lastmenux;    }    else check->lastmenux->nextmenux = NULL;  else if (check->nextmenux)  {    check->nextmenux->lastmenux = NULL;    menus = check->nextmenux;  }  else    menus = NULL;      check->Release(); //see below  delete check;  check = NULL;}void MenuX::Release(){  SubMenu *checkmenu = NULL, *tempmenu = NULL;  MENUITEM *checkitem = NULL, *tempitem = NULL;    if (submenus)  {    checkmenu = submenus;    while (checkmenu)    {      while (checkmenu->submenus)        checkmenu = checkmenu->submenus;      if (checkmenu->items)        checkitem = checkmenu->items;            while (checkitem)      {        tempitem = checkitem->nextitem;        delete checkitem;        checkitem = tempitem;      }      if (checkmenu->parent->submenus)        checkmenu->parent->submenus = checkmenu->parent->submenus->nextmenu;      if (checkmenu == this && checkmenu->parent == this)        tempmenu = NULL;      else tempmenu = checkmenu->parent;      if (drawfont)        drawfont->RemoveText(checkmenu->name);      checkmenu->parent = NULL;      checkmenu->items = NULL;      checkmenu->submenus = NULL;      checkmenu->nextmenu = NULL;      checkmenu->lastmenu = NULL;      delete checkmenu;      checkmenu = tempmenu;    }  }  else if (items)  {    checkitem = checkmenu->items;          while (checkitem)    {      tempitem = checkitem->nextitem;      delete checkitem;      checkitem = tempitem;    }  }  delete [] allmenus;  drawfont = NULL;  if (drawimage)    drawimage->Release();  if (pinst)    pinst->Release();  drawimage = NULL;  pinst = NULL;  parent = NULL;  currentmenu = NULL;}

now i know how confusing the MenuX::Release function is, but it works (thats why the program doesn't crash upon releasing the menu). "menus" is a pointer to the start of a linked list of all menus that have been loaded. so when a menu (or any object for that matter) is created or removed, it is added to or removed from lists like this. the part i don't get is how this can cause the menu loading function to screw up the second time round. nothing is or can be connected to the 2 classes or their variables, so removing a menu shouldn't cause anything to go wrong with loading.... but it seems to...

i know this is a very long post, but it seemed my last one wasn't informative enough so i want to make sure this one is [smile].
if anyone can help me with this, please give me some advice because i'm extremely puzzled by this.

thanks in advance,

jflanglois

Apr 13, 2006 at 7:34 AM

I haven't really read the code in detail, but I think you should consider using a smart pointer like the shared_ptr of Boost.Smart Pointers to manage the lifetime of your menu objects, because you are most likely deleting an object that you are then trying to use.

Also, you should use std::list instead of a homegrown linked list. It will make the manipulation that much easier. And no, "overhead" is not a valid argument.

jfl.

TheGangster

Apr 13, 2006 at 8:02 AM

I believe that you are trying to delete pointers which were not allocated by you.

Not every pointer needs a delete, only _WHEN_ you allocate memory (malloc/new), so you will have to count the new/malloc and check below if the counting is the same (which probably is not).

Also you might want to consider start using macros to delete/release these are from DirectX SDK you can use them whenever you want.

#define SAFE_DELETE(p)  { if(p) { delete (p);     (p)=NULL; } }#define SAFE_RELEASE(p) { if(p) { (p)->Release(); (p)=NULL; } }#define SAFE_DELETE_ARRAY(p) { if(p) { delete[] (p);   (p)=NULL; } }

Hope this will help you.

stylin

Apr 13, 2006 at 3:13 PM

Quote:
Original post by TheGangster
Not every pointer needs a delete, only _WHEN_ you allocate memory (malloc/new), so you will have to count the new/malloc and check below if the counting is the same (which probably is not).

Resources acquired from external libraries may need to be freed manually as well. The corresponding documentation will state how resources are to be dealt with.

I would also advise against delete macros. Memory management can be a difficult and time consuming process, heightened by the abstraction of macros littered about.

the_moo

Author

the_moo

Apr 13, 2006 at 8:49 PM

ok, i've got the problem fixed now!
thanks jflanglois, i took you're advice purely because i couldn't see why it would hurt (besides having to change a bunch of code...) and all is working great now! and the code is quiet a lot simpler too. thanks again [smile].
as for the whole memory thing, the only external libraries i'm using are those in the directx api, so all of that needn't be worried about.

anyway, all is working now so thanks again for your advice/pointing me in the right direction [wink].

jflanglois

Apr 13, 2006 at 9:10 PM

You're welcome. Glad you got it all working.

Topic Locked

This topic has been locked by a moderator. New replies are not allowed.