• Status: Solved
  • Priority: Medium
  • Security: Public
  • Views: 235
  • Last Modified:

How do I delete a varible I'm returning?

I'm trying to kill the memory leaks. One thing that bugs me though is i have 2 gbs of ram and this program has memory alloc errors and task manager says its only taking up 3mbs. I've killed some of them by deleting some of the big stuff and by making curtain variables static. Basically, I'm trying to go though trying to find anything that uses new, and put it when a delete.

Okay, my question is how do I delete ret? Or will it be freed up on it's own? It's just getting passed to another function.

This function does two things it converts rt to a char. And it cuts off when it finds 3 consecutive spaces.

I'm using VC++ 8.0 Express.

char *trim(unsigned short * rt)
{
      unsigned char i=0;
      unsigned char len = 0;
      union u {unsigned short int vi; char c;};
      union u un;
      while(!((char)rt[len] == 32 && (char)rt[len+1] == 32 && (char)rt[len+2] == 32 || (char)rt[len] == 0))
      {
            len ++;
      }
      char * ret = new char[len];
      while(i<len){
            un.vi = rt[i];
            ret[i++] = un.c;
      }
      ret[i]=0;
      return ret;
}

std::string GAMEVARS::desc(void)
{
      static const char *el = "\r\n";
      static const char *Level[5] = {LDMSG_VERYLOW,LDMSG_LOW,LDMSG_MEDIUM,LDMSG_HIGH,LDMSG_VERYHIGH};
      static const char *Bool[2] = {"False","True"};
      descbuff.clear();
      descbuff.str("");
      descbuff << trim(m_desc.Mapname)<< el << trim(m_desc.Description) << el <<
            GDMSG_HOST << m_hostedby << el << 
            GDMSG_DEATHMATCH << Bool[m_desc.Deathmatch?1:0] << el <<
            GDMSG_MAXPLAY << (unsigned short)m_maxPlayers << el << 
            GDMSG_HOLDTIME << (unsigned short)m_desc.Holdtime << el <<
            GDMSG_TIMELIMIT << m_timelimit << el <<      
            GDMSG_POWERUP << Bool[m_desc.Powerups?1:0] << el <<
            GDMSG_RECHARGE << Level[(unsigned short)m_desc.Rechargerate] << el <<
            GDMSG_LASER << Level[(unsigned short)m_desc.Laserdamage] << el <<
            GDMSG_SPECIAL << Level[(unsigned short)m_desc.Specialdamage] << el << 
            GDMSG_MISSILE << Bool[m_desc.Missilies?1:0] << el << 
            GDMSG_GRENADE << Bool[m_desc.Bombs?1:0] << el << 
            GDMSG_BOUNCY << Bool[m_desc.Bouncies?1:0] << el << 
            GDMSG_MINE << Bool[m_desc.Mines?1:0] << el << 
            GDMSG_FLAME << Bool[m_desc.Flame?1:0];
      return descbuff.str();
}
0
pcvii
Asked:
pcvii
2 Solutions
 
jkrCommented:
That would just require adding
std::string GAMEVARS::desc(void)
{
      static const char *el = "\r\n";
      static const char *Level[5] = {LDMSG_VERYLOW,LDMSG_LOW,LDMSG_MEDIUM,LDMSG_HIGH,LDMSG_VERYHIGH};
      static const char *Bool[2] = {"False","True"};
      descbuff.clear();
      descbuff.str("");
      char* mymap = trim(m_desc.Mapname); // <--------------------
      descbuff << mymap << el << trim(m_desc.Description) << el <<
            GDMSG_HOST << m_hostedby << el <<
            GDMSG_DEATHMATCH << Bool[m_desc.Deathmatch?1:0] << el <<
            GDMSG_MAXPLAY << (unsigned short)m_maxPlayers << el <<
            GDMSG_HOLDTIME << (unsigned short)m_desc.Holdtime << el <<
            GDMSG_TIMELIMIT << m_timelimit << el <<     
            GDMSG_POWERUP << Bool[m_desc.Powerups?1:0] << el <<
            GDMSG_RECHARGE << Level[(unsigned short)m_desc.Rechargerate] << el <<
            GDMSG_LASER << Level[(unsigned short)m_desc.Laserdamage] << el <<
            GDMSG_SPECIAL << Level[(unsigned short)m_desc.Specialdamage] << el <<
            GDMSG_MISSILE << Bool[m_desc.Missilies?1:0] << el <<
            GDMSG_GRENADE << Bool[m_desc.Bombs?1:0] << el <<
            GDMSG_BOUNCY << Bool[m_desc.Bouncies?1:0] << el <<
            GDMSG_MINE << Bool[m_desc.Mines?1:0] << el <<
            GDMSG_FLAME << Bool[m_desc.Flame?1:0];
      delete [] mymap; // <---------------------------------------------
      return descbuff.str();
}

to store the return value and later delete it instead of directly passing it to the output stream.
0
 
abithCommented:
(1) Almost all standard string manipulation functions will return pointer to the one of its parameters. So either change the parameter and return the starting postion or send one more char* parameter, change it internally and return the same.

(2) make a class, create method called trim. use the private temp variable, use this for creating return values, remove the temp varriable in destructor if its not NULL.

(3) follow jkr's step.

and, out of question!!. its good pactice to use pointer instead of array index while using pointer varriable.

char *trim(unsigned short * rt)
{
      int len = 0;       
      int space = 0;
      unsigned short *rv = rt;
      while (!(*rt == 32 && *(rt+1)==32 &&  *(rt+2)==32) && *(rt++) ) // Are you sure, that you need only trim to be proceeded only incase of three occurance of spaces!!!
               len++;

      char * ret = new char[len +1];
      ret [len] = NULL;

      while (len -- )
        *(ret + len ) =  (char)*(--rt) ;
      return ret;
}
 doesn't look good? :)
0
 
Infinity08Commented:
From jkr's code : do the same for the other trim in this line :

      descbuff << mymap << el << trim(m_desc.Description) << el <<
0
What does it mean to be "Always On"?

Is your cloud always on? With an Always On cloud you won't have to worry about downtime for maintenance or software application code updates, ensuring that your bottom line isn't affected.

 
pcviiAuthor Commented:
I've been rewriting this lobby server. and the game servers send a structure of data all at once. loaded with spaces and no null terminators. I was thinking 3 spaces for now would be good for test. I can change it later if I need to. maybe I should just work my way from the end. *shrugs* :P

You guys all helped. I'll do the method jkr was suggesting for now. might change the function around to be like abrith was talking about later.

thanks.
0
 
pcviiAuthor Commented:
what am i saying arith's version of the function is much nicer than mine. in it goes :P
0
 
rstaveleyCommented:
Minimum edit fix is to use std::string as a temporary object to return from trim and to delete the temporary string allocated within trim. Cleanup for std::string gets done automatically for you and your "hand coding" is confined to the scope of trim.


std::string trim(unsigned short * rt)  
{
      unsigned char i=0;
      unsigned char len = 0;
      union u {unsigned short int vi; char c;};
      union u un;
      while(!((char)rt[len] == 32 && (char)rt[len+1] == 32 && (char)rt[len+2] == 32 || (char)rt[len] == 0))
      {
            len ++;
      }
      char * ret = new char[len];
      while(i<len){
            un.vi = rt[i];
            ret[i++] = un.c;
      }
      ret[i]=0;
//--------8<--------
//    return ret;
//--------8<--------
      std::string str(ret);
      delete []ret;
      return str;
//--------8<--------
}


A better approach would be to make ret an std::ostringstream, which you insert into in the loop, and return the std::string ret.str(). That way you loose the new and delete.

If unsigned short* is really a wchar_t*, you could even use a std::wostringstream and return a std::string, using a WideCharToMultiByte() conversion.

For what it's worth, I disagree with abith about the pointer. It is more legible to index an array than use pointer arithmetic, I reckon, and the generated code will be just as efficient with a modern compiler.


I wonder if unsigned short  is really a wchar_t*? If so
0

Featured Post

Industry Leaders: We Want Your Opinion!

We value your feedback.

Take our survey and automatically be enter to win anyone of the following:
Yeti Cooler, Amazon eGift Card, and Movie eGift Card!

Tackle projects and never again get stuck behind a technical roadblock.
Join Now