Member Avatar for samweb$ 
if(m_playSound)
{
	const char* soundToPlay = layout->GetWinName();
	char* soundTemp = NULL;				

if(m_playAltScatterSndSymbol != -1)
{
for(int i = 0; i < layout->GetNumPositions(); i++)
{
  ReelWinSymbolPosition* rwsp = layout->GetReelSymbolPosition(i);

   for(int i = 0; i < layout->GetNumPositions(); i++)
    {
  ReelWinSymbolPosition* rwsp = layout->GetReelSymbolPosition(i);

 if(rwsp && rwsp->GetSymbol() == m_playAltScatterSndSymbol)
    {
   if(m_alternateScatterSndSymbolName && m_alternateScatterSndSymbolName[0] != '\0')
   {
CHK_NEW(soundTemp = new char[strlen(m_alternateScatterSndSymbolName) + strlen(soundToPlay)+1])

memset(soundTemp,0,sizeof(soundTemp));
strcpy(soundTemp,m_alternateScatterSndSymbolName);
strcat(soundTemp,soundToPlay);
    }
else
return g_log.Error("BlurredReelsController::AnimatingScatter: Alternative scatter sound is empty");
     }
   }
 }

m_playAltScatterSndSymbol = -1;
}


if(soundTemp == NULL)
PlayWinSound(soundToPlay);
else
{
PlayWinSound(soundTemp);
CHK_DELETE_ARRAY(soundTemp);   //CHK_DELETE_ARRAY is a #define as delete [] soundTemp; soundTemp= NULL
}

Hi,

Can anyone help me in fixing the memory leak in the above code, the leak is at line number 20 (soundTemp), the
CHK_DELETE_ARRAY(soundTemp) delete the soundTemp pointer, still i see some memory leaks.

Please help !!!

Thanks
SAM

Edited by samweb$ because: n/a
  • 2 Contributors
  • 1 Reply
  • 87 Views
  • 8 Hours Discussion Span
  • Latest Post Latest Post by Salem
Member Avatar for Salem

1. How do you know there is a memory leak here?

2. The code indentation is a mess.

if(m_playSound)
{
  const char* soundToPlay = layout->GetWinName();
  char* soundTemp = NULL;

  if(m_playAltScatterSndSymbol != -1)
  {
    for(int i = 0; i < layout->GetNumPositions(); i++)
    {
      ReelWinSymbolPosition* rwsp = layout->GetReelSymbolPosition(i);

      for(int i = 0; i < layout->GetNumPositions(); i++)
      {
        ReelWinSymbolPosition* rwsp = layout->GetReelSymbolPosition(i);

        if(rwsp && rwsp->GetSymbol() == m_playAltScatterSndSymbol)
        {
          if(m_alternateScatterSndSymbolName && m_alternateScatterSndSymbolName[0] != '\0')
          {
            CHK_NEW(soundTemp = new char[strlen(m_alternateScatterSndSymbolName) + strlen(soundToPlay)+1])

            memset(soundTemp,0,sizeof(soundTemp));
            strcpy(soundTemp,m_alternateScatterSndSymbolName);
            strcat(soundTemp,soundToPlay);
          }
          else
          { /* added { */
            return g_log.Error("BlurredReelsController::AnimatingScatter: Alternative scatter sound is empty");
          } /* added } */
        }
      }
    }

    m_playAltScatterSndSymbol = -1;
  }

  if(soundTemp == NULL)
  { /* added { */
    PlayWinSound(soundToPlay);
  } /* added } */
  else
  {
    PlayWinSound(soundTemp);
    CHK_DELETE_ARRAY(soundTemp);   //CHK_DELETE_ARRAY is a #define as delete [] soundTemp; soundTemp= NULL
  }

If you're trying to resolve things like memory leaks, then knowing the scope of things is vital. Working out scope is a hell of a lot easier if the code is nicely indented to begin with.

3. This is C++, so why are you messing with unsafe C-style char arrays, allocation and copy functions?

4. memset(soundTemp,0,sizeof(soundTemp));
This is at best useless, and at worst a memory corruption problem.
What you obviously intended to do was clear the whole buffer, but you're just clearing the sizeof pointer (not the amount it points to).
Further, because you follow with strcpy/strcat, the effect of the memset, even if it were correct, is momentary.

Be a part of the DaniWeb community

We're a friendly, industry-focused community of developers, IT pros, digital marketers, and technology enthusiasts meeting, networking, learning, and sharing knowledge.