Thread: Problem with the socket

I am trying to write a pop3 fetcher module on windows. Below is the code, which receives the data from the connected socket.

It takes three parameters as input buffer, socket and data termination string.

The problem is whenever the data received on the socket is more then the DEFAULT_BUFFER it hangs.

Code:

int
GetCommandData (char **szBuffer, SOCKET * scl, char cmp[])
{
	char *ptr=NULL;
	int ret,idx=1;
	char recvBuffer[DEFAULT_BUFFER];
	do{
		free(*szBuffer);
        	ret = recv (*scl, recvBuffer, DEFAULT_BUFFER - 1, 0);
		if (ret == SOCKET_ERROR || ret == 0)
			return POP3_RECV_FAILED;
		recvBuffer[ret] = '\0';
		idx += strlen(recvBuffer);
		*szBuffer=malloc(idx);
		if (*szBuffer == NULL)
			return POP3_MEMORY_ALLOC_FAILED;
		if(ptr != NULL)
		{
			strcpy(*szBuffer,ptr);
			strcat(*szBuffer,recvBuffer);
			free(ptr);
		}else{
			strcpy(*szBuffer,recvBuffer);	
		}
		ptr = *szBuffer;
	}while(strcmp(&recvBuffer[strlen(recvBuffer) - strlen(cmp)],cmp) != 0);	
	return POP3_SUCCESS;
}

You always free the buffer at the start of the loop which is a problem because ptr is pointing to it at the time. Then later in the loop you try copying the data that ptr points to (which has been freed). You might want to try realloc()'ing szBuffer instead of free()'ing it and then malloc()'ing it every time. Then you can completely do away with ptr.

If you cut all the garbage you'll see what I mean:

Code:

	do{
		free(*szBuffer);
		*szBuffer=malloc(idx);
		if(ptr != NULL)
		{
			strcpy(*szBuffer,ptr);
			free(ptr);
		}
		ptr = *szBuffer;
	}while(strcmp(&recvBuffer[strlen(recvBuffer) - strlen(cmp)],cmp) != 0);

Look at it from starting near the end of the first loop and you can see you've got something like this:

ptr = *szBuffer;
free(ptr);
strcpy(*szBuffer, ptr);
free(ptr);

You can't use memory once it's been free()'d (not to mention free() it twice).

Remember that:

Code:

void *ptr1, *ptr2;

ptr1 = malloc(50);
ptr2 = ptr1;
free(ptr2);

is the same as:

Code:

void *ptr1;

ptr1 = malloc(50);
free(ptr1);

Thanks for the reply

Now I have removed free at start of the loop.Here is the caller

Code:

int
get_pop3_Message (pop3_account pa, int (*callback) (int, char *))
{
  SOCKET sClient;
  struct sockaddr_in server;
  char outbuffer[DEFAULT_BUFFER];
  char *inbuffer;
  sClient = socket (AF_INET, SOCK_STREAM, IPPROTO_TCP);
  inbuffer = NULL; 
  if (sClient == INVALID_SOCKET)
    {
      return POP3_SOCKET_FAILED;
    }
  if (DSNResolution (&server, pa.pop_server, pa.pop3_port) ==
      POP3_HOST_RES_FAILED)
    return POP3_HOST_RES_FAILED;
  if (connect (sClient, (struct sockaddr *) &server, sizeof (server)) ==
      SOCKET_ERROR)
    {
      return POP3_CONNECT_FAILED;
    }
  GetCommandData (&inbuffer, &sClient, "\r\n");
  printf ("-><-%s\n",inbuffer);
  free(inbuffer);
  sprintf (outbuffer, "user %s \r\n", pa.user);
  SendCommandData (outbuffer, &sClient);
  GetCommandData (&inbuffer, &sClient, "\r\n");
  printf ("-><-%s\n", inbuffer);
  free(inbuffer);
  sprintf (outbuffer, "pass %s \r\n", pa.pass);
  SendCommandData (outbuffer, &sClient);
  GetCommandData (&inbuffer, &sClient, "\r\n");
  printf ("-><-%s\n", inbuffer);
  free(inbuffer);
  sprintf (outbuffer, "stat \r\n", pa.user);
  SendCommandData (outbuffer, &sClient);
  GetCommandData (&inbuffer, &sClient, "\r\n");
  printf ("-><-%s\n", inbuffer);
  free(inbuffer);
  if (pa.id == 1)
    {
      printf("Hit 1\n");
      sprintf (outbuffer, "retr 1\r\n", pa.user);
      printf("Hit 2\n");
      SendCommandData (outbuffer, &sClient);
      printf("Hit 3\n");
      GetCommandData (&inbuffer, &sClient, "\r\n.\r\n");
      printf("Hit 4\n");
      printf ("%d\n%s\n", strlen (inbuffer), inbuffer);
	  free(inbuffer);
    }
  return POP3_SUCCESS;

}

But it is not printing the "hit 4 " print statment. And also when I am debugging it works . I am using VC6 on Winxp sp2. config data is stored in www.sqlite.org db. I am attaching the entire source with it for the reference.

thanks