Memory allocation error

Started by
4 comments, last by zackriggle 20 years, 3 months ago
I seem to be getting an error with the call of ''new'' in the following function:

// Waits on a socket and received data on the specified

// socket until the connection closes, the buffer is filled,

// or a CRLF is received. Yes, I know I am not using select()

// like I should. Oh well. BACKSPACE is defined as whatever

// ASCII code the Backspace key gets. The same for LINEFEED.

int WaitForReply( SOCKET s, char *b, WORD len )
{
  if( b   == NULL ||
      len == 0      )
    return (~0);

  memset(b,0,len);
  char  *buffer;
    buffer = new char[len]; // <-- ERROR HERE!!! ------------

  ZeroMemory(buffer, len); // <-- This was added after-the-fact, not the cause.

  int    r      = 0;        // Return value

  bool  bLoop   = true;     // Loop while this is true

  int    bufus  = 0;        // Next ununsed byte in the buffer

  char  *lf     = NULL;     // Will point to a LINEFEED if there is any


  while( bLoop )
  {
    // This setup will start writing data to the first free byte in our buffer,

    // and will NOT receive more data than the buffer will hold (but it will go

    // up to the max-1 to leave space for \0 ).

    r = recv( s, &buffer[bufus], (len-1) - bufus, 0 );

    // The socket has gracefully closed, return an error (NOT data).

    if( r == 0 )
      break;

    // R holds the number of bytes received; add that amount to ''bufus''.

    bufus += r;

    // Remove backspace characters, and the preceding character

    for(int i = 0; i <= bufus; i++ )
    {
      if( buffer[i] == BACKSPACE &&
          i > 0 ) // Do not perform backspace if there are no charaacters to erase.

      {
        memmove(&buffer[i-1], // Move data to the character BEFORE the backspace

          &buffer[i+1],     // Start moving AFTER the backspace

          (bufus-i)+1);     // Move TotalLen - BackspaceIndex characters

        // plus one character

        // Diagram:

        //

        // "Hello My Nm\bame is Zach"

        //             ^-backspace

        // Move "ame is Zach" over ''m''

        // "Hello My Nm\b" <-- last two characters overwritten

        //       "ame is Zach"


        bufus -= 2; // We eliminated 2 characters from the buffer


      }
    }

    // Check to see if we have reached our limit (use len-1 because of null terminator)

    if( bufus == (len-1) )
    {
      memcpy(b,buffer,(len - 1) - strlen(LINEFEED));    // Copy the buffer, excluding the LINEFEED.

                                                        // We do not want a LINEFEED in ''b''.

      //           - ?

      memset(&b[bufus-strlen(LINEFEED)],0,1);           // Add null terminator where the first character

                                                        // of the linefeed WAS.

      delete [] buffer;
      return bufus;
    }

    // Check for a LINEFEED

    lf = strstr( buffer, LINEFEED );
    if( lf != NULL )
    {
      // We found a linefeed.


      // Subtract the address of lf from the address of buffer to give us the length

      // of THIS message, excluding the next.

      // I.E. (0x00485F85) - 0x00485F73 = 0x11 (17 bytes)

      // "do some command!!\n\rdo some other command"

      //  \               /^ ^ ^------v

      //   \_____________/ | 19th     20th

      //       17 bytes    18th byte in buffer

      int msgLen = (((DWORD)lf)) - ((DWORD)buffer);

      // Copy #bufus bytes from the buffer to ''b''.

      memcpy(b, buffer, msgLen);

      // Add the null terminator

      memset(&b[msgLen], 0, 1);

      // Free our buffer

      delete [] buffer;

      // Return

      return msgLen;
    }
  }
  delete [] buffer;
  buffer = NULL;
  return (~0);
} //WaitForReply()

My (soon-to-be) Cherished Cookie of Appreciation: -- MattB - WinSock advice -- -- groby - safely assuming max MTU size -- -- SiCrane - bug with CREATE_SUSPENDED on accident -- -- Voxelsoft - buffer overruns with itoa()
Advertisement
I notice that you are assuming that recv will succeed. If it fails, you will get a negative return value. Then your code will blindly go about using a negative index into your memory buffer. That could definitely lead to bad things...
Yes, that is assumed. Data should either be recv'd or the connection closes. I suppose on some rare event it might have an error (adds error handling).

Edit:
The error occurs seemingly randomly, with no changing variables.

[edited by - zackriggle on January 14, 2004 6:02:23 PM]
When you say "error here" do you mean seg fault, exception, or something different?

edit: on a completely unrelated note, didn't Sneftel have a cookie of appreciation too?

[edited by - SiCrane on January 14, 2004 6:25:04 PM]
Dunno, maybe. I do remember removing a cookie that kind of make me look dumb (might have been Sneftel's). The error seems to be a ghost now (it had something to do with the heap... (heap_chk or something)). I can't reproduce it and it has stopped happening. Until it happens again (hopefully it won't ), I'll just keep working on other things. I'll write all the stuff down if it does.

New task for the night: a CTCP parser :D.

Edit:
Yep, Sneftel helped me with 'extern' (and someone who has to ask about a keyword has to be either dumb, dumb, or both :D). Hence, it was removed.

My (soon-to-be) Cherished Cookie of Appreciation:
-- MattB - WinSock advice --
-- groby - safely assuming max MTU size --
-- SiCrane - bug with CREATE_SUSPENDED on accident --
-- Voxelsoft - buffer overruns with itoa()

[edited by - zackriggle on January 14, 2004 7:08:10 PM]
What might have happened was that in unrelated code you were deleting/freeing a bad pointer, which killed the heap consistency, but the runtime didn''t notice until you called new later. So if code somewhere else changed, it might have fixed it without you realizing it.

This topic is closed to new replies.

Advertisement