Memory Error - Recv() Thread

[Spilled]

I'm recieving an error in my recv() thread. I'm not sure if im going about this the right way so ill show you everything i got. When i go File->Connect:

Code Select Expand


                 case FILE_CONNECT: //CONNECT EVENT
                 {
                     s << cWhite << s.getTime() << cYellow << "Attempting to Connect to Battle.net\n";
                     if(is_connected == true)
                     {
                         closesocket(wSock);
                         is_connected = false;
                     }
                     if(open(Server,6112) == -1)
                     {
                         s << cWhite << s.getTime() << cRed << "Connection failed...\n";
                     }
                     else
                     {
                         is_connected = true;
                         threadHwnd = CreateThread(NULL,0,&ThreadProc,(LPVOID)SOCKET_RECIEVE,0,NULL);
                         s << cWhite << s.getTime() << cGreen << "Connected!\n";
                         s << cWhite << s.getTime() << cYellow << "Sending SID_AUTH_INFO\n";
                                p.InsertDWORD(0);
                                p.InsertNonNTString( "68XIPXES" );
                                p.InsertDWORD( 0xCD );
                                p.InsertDWORD(0);
                                p.InsertDWORD(0);
                                p.InsertDWORD(0);
                                p.InsertDWORD(0);
                                p.InsertDWORD(0);
                                p.InsertNTString("USA");
                                p.InsertNTString("United States");
                                if(is_connected)
                                     p.SendBNCSPacket(wSock,0x50);
                      }
                 }
                 break;


There I created a thread to make a continuous loop for my recv() until connection is closed or an error occurs:

Heres the proc:

Code Select Expand


DWORD WINAPI ThreadProc(LPVOID lpParameter)
{
      switch((int) lpParameter)
      {
           case SOCKET_RECIEVE:
                        {
                               int iResult = 0;
                               char Header[4];
                               do{
                                    iResult = recv(wSock,Header,4,MSG_PEEK);
                                    if(iResult > 0)
                                    {
                                               //s << cWhite << s.getTime() << cGreen << "Data Available!\n";
                                               if(iResult > 3) //has the packet head
                                               { 
                                                   //MessageBox(NULL,"TEST","",MB_OK);
                                                   int pLen = Header[2];
                                                   char buf2[2];
                                                   //memmove(dest,src,count)
                                                   char FullPacket[pLen+1];
                                               
                                                   if(iResult >= pLen)
                                                   {
                                                   //the Full packet is in the buffer
                                                   
                                                   recv(wSock,FullPacket,pLen,0);
                                                   Parsep(FullPacket,pLen);
                                                             
                                                   }else{
                                                       //Loop until the full packet is in the buffer
                                                       int t = recv(wSock,FullPacket,pLen,MSG_PEEK);
                                                       while(!t >= pLen)
                                                       {
                                                                t = recv(wSock,FullPacket,pLen,MSG_PEEK);
                                                       }
                                                       
                                                       recv(wSock,FullPacket,pLen,0);
                                                       Parsep(FullPacket,pLen);
                                                       
                                                   }
                                                   
                                               }
                                    }else{
                                          if(iResult == 0)
                                          {
                                               //Connection Closed. 
                                               s << cWhite << s.getTime() << cRed << "Connection Closed.\n";
                                               closesocket( wSock );
                                               is_connected = false;
                                               WSACleanup();
                                          }else{
                                                //call WSAGetLastError
                                               s << cWhite << s.getTime() << cRed << "(Call WSAGetLastError () )Connection Closed.\n";
                                               closesocket( wSock );
                                               is_connected = false;
                                               WSACleanup();
                                          }
                                    }
                                 } while(iResult > 0);                                   
                        }           
                        break;   
      }
}


I'm currently working on 0x51 so it just stops when I recieve 0x50 back but that still shouldn't create an error. The recv() thread should continuously loop until connection is closed or an error occurs then display the error and do WSACleanup(). Am I going about this the wrong way? or is there a bug I dont see... I contacted UL via PM about this the other day but I thought i would create a topic so others could learn from my mistakes. Thanks in advance guys!

[Kp]

What error are you receiving?  Access violation, recv error, socket closes, ...?  Also, don't do while (!t >= pLen).  It's almost certainly not what you want.

[Spilled]

Error: The memory could not me "written"

What would you suggest instead of the while() loop?

[UserLoser]

int pLen = Header[2]; // this isn't what you want

That'll take the 3rd byte in header and place it into a 32bit integer named pLen.  Like I said, you need to do something like: int pLen = *(int*)(Header+2); to get the appropriate value

Also, don't forget to send the protocol type byte 0x1 to Battle.net before you send SID_AUTH_INFO, e.g., send(socket, (const char*)"\x1", 1, 0);

[Kp]

If it's an access violation, just build with symbols, run, and pop it in a debugger when it crashes.  Easy.

Incidentally, that's an absolutely terrible loop.  It will peg your processor while waiting for incoming data.  Most modern games seem to get away with maxing out the CPU, but that behavior simply isn't tolerated from anything else.

[Spilled]

int pLen = Header[2]; // this isn't what you want

That'll take the 3rd byte in header and place it into a 32bit integer named pLen.  Like I said, you need to do something like: int pLen = *(int*)(Header+2); to get the appropriate value

Also, don't forget to send the protocol type byte 0x1 to Battle.net before you send SID_AUTH_INFO, e.g., send(socket, (const char*)"\x1", 1, 0);

what you showed me was giving me off the wall numbers like 23404 and -50, this is the only way that was giving me the correct integer value. The protocol byte i add in my buffer class

Code Select Expand


void buffer::SendBNCSPacket(SOCKET wSock, BYTE id)
{
    if(id == 0x50)
    {
          char Packet[pos+5];
          Packet[0] = 0x01;
          Packet[1] = 0xFF;
          Packet[2] = id;
          Packet[3] = (WORD)pos+4;
          Packet[4] = 0x00;
          int ff = 5;
          for(int i = 0;i < pos;i++)
          {
                  Packet[ff] = Buffer[i];
                  ff++;
          }
          int bytessent = send(wSock,Packet, pos+5,0);
          pos = 0;
    }else{
          char Packet[pos+4];
          Packet[0] = 0xFF;
          Packet[1] = id;
          Packet[2] = (WORD)pos+4;
          Packet[3] = 0x00;
          int ff = 4;
          for(int i = 0;i < pos;i++)
          {
                  Packet[ff] = Buffer[i];
                  ff++;
          }
          send(wSock,Packet,pos+4,0);
          pos = 0;
    }
}


I packet logged my 0x50 and this was correct. It had the protocol byte before the packet and everything. I have no idea what the problem is. Anymore ideas?

Heres the open method i call in my file->connect that i failed to add in my previous post:

Code Select Expand


int open(char* addr, int p)
{
   WORD wVersionRequested = MAKEWORD(1,1);
WSADATA wsaData;
int nRet = WSAStartup(wVersionRequested, &wsaData);
if (wsaData.wVersion != wVersionRequested)
{
return -1;
}
   LPHOSTENT lpHostEntry;

lpHostEntry = gethostbyname(addr);
   if (lpHostEntry == NULL)
   {
       return -1;
   }
wSock = socket(AF_INET, // Address family
  SOCK_STREAM, // Socket type
  IPPROTO_TCP); // Protocol
if (wSock == INVALID_SOCKET)
{
return -1;
}
SOCKADDR_IN saServer;

saServer.sin_family = AF_INET;
saServer.sin_addr = *((LPIN_ADDR)*lpHostEntry->h_addr_list);
// ^ Server's address
saServer.sin_port = htons(p);

   nRet = connect(wSock, // Socket
  (LPSOCKADDR)&saServer, // Server address
  sizeof(struct sockaddr));
   delete addr;
   if(nRet == SOCKET_ERROR)
           return -1;
   else
           return 0;    
}


Anymore ideas guys? I appreciate your help so far

kp: Im using Dev-C++ and honestly I have never messed with there debugger there for i have no experience with it. As for the loop I'll work on that right now

[Spilled]

Sorry about the double post. Kp I rewrote part of the thread:

How about this?

Code Select Expand


DWORD WINAPI ThreadProc(LPVOID lpParameter)
{
      switch((int) lpParameter)
      {
           case SOCKET_RECIEVE:
                        {
                               int iResult = 0;
                               char Header[4];
                               do{
                                    iResult = recv(wSock,Header,4,MSG_PEEK);
                                    if(iResult > 0)
                                    {
                                               //s << cWhite << s.getTime() << cGreen << "Data Available!\n";
                                               if(iResult > 3) //has the packet head
                                               { 
                                                   int pLen = Header[2];
                                                   //memmove(dest,src,count)
                                                   
                                                   char FullPacket[pLen+1];
                                                   
                                                   if((recv(wSock,FullPacket,pLen,MSG_PEEK)) >= pLen)
                                                   {
                                                   //the Full packet is in the buffer
                                                   
                                                   recv(wSock,FullPacket,pLen,0);
                                                   Parsep(FullPacket,pLen);
                                                             
                                                   }
                                                   
                                               }
                                    }else{
                                          if(iResult == 0)
                                          {
                                               //Connection Closed. 
                                               s << cWhite << s.getTime() << cRed << "Connection Closed.\n";
                                               closesocket( wSock );
                                               is_connected = false;
                                               WSACleanup();
                                          }else{
                                                //call WSAGetLastError
                                               s << cWhite << s.getTime() << cRed << "(Call WSAGetLastError () )Connection Closed.\n";
                                               closesocket( wSock );
                                               is_connected = false;
                                               WSACleanup();
                                          }
                                    }
                                 } while(iResult > 0);                                   
                        }           
                        break;   
      }
}


When I recieve my reponse from 0x50, I dont send 0x51 because i Havent got to that part yet. Im trying to fix the problem im having with this recv() thread. After I recieve 0x50 It errors out when It is suppose to keep looping and waiting for incoming data.... I really need some help with this here so all input is appreciated

[Kp]

That's slightly better, but there's still a potential spinloop if the peer does not send you a full message.  Additionally, you're calculating the message size incorrectly, so large packets will be mangled rather badly.

[Spilled]

Honestly Kp I've sat here and tried a lot of different things. Perhaps a sample or some help via AIM? That would be greatly appreciated...

[UserLoser]

Actually, don't use an int for the message size, try a short.  short pLen = *(short*)(Header+2);  I guess sizeof(int) for you is 4 so that's why you were getting bad values.

[Kp]

I don't use AIM and I don't have time to write an example for you.  It's a choice between my brief comments (which apparently are not sufficiently helpful) or me saying nothing at all.  Sorry.

[Win32]

int pLen = Header[2];

Could be your problem, as mentioned above.

The length field of Battle.net headers is an unsigned 16-bit value (unsigned short), your compiler is generating code to interpret it as a signed 32-bit value (int).

You have two options;
#1: Declare 'pLen' as an unsigned short instead.
#2: Inform the compiler to interpret the header value properly. " int pLen = *((Word*) &Header[2]) "

But, I would suggest you just take the time to write a header structure, like;

Code Select Expand


struct __declspec(align(0)) BnetMsgHeader
{
Byte Head;      /* Message head. Always 0xFF.  */
Byte MessageId; /* Message purpose identifier. */
Word wLength;   /* Message length, in bytes.   */
};



Then all you would have to do is declare 'Header' as a BnetMsgHeader, and access the 'wLength' member when assigning pLen.


-Matt