[C++] change function implementation

[THEARTOFWEB]

Hi,

I would like to turn this function:

std::string Socket::ReceiveBytes() {
  std::string ret;
  char buf[1024];
 
  while (1) {
    u_long arg = 0;
    if (ioctlsocket(s_, FIONREAD, &arg) != 0)
      break;

    if (arg == 0)
      break;

    if (arg > 1024) arg = 1024;

    int rv = recv (s_, buf, arg, 0);
    if (rv <= 0) break;

    std::string t;

    t.assign (buf, rv);
    ret += t;
  }
 
  return ret;
}

into something like:

bool Socket::ReceiveBytes(char * buffer, const int buffersize);

how should I change it internally?

thanks

[Skizmo]

how should I change it internally?

Give it your best try... if you fail, post your problem.

[THEARTOFWEB]

this is my best try...untested though:

#include <algorithm>

bool Socket::ReceiveBytes(char * buffer, const int buffersize)
{
	char temp[1024];

	while (1)
	{
		u_long arg = 0;

		if (ioctlsocket(s_, FIONREAD, &arg) != 0)
			return false;

		if (arg == 0)
			return true;

		if (arg > 1024)
			arg = 1024;

		int rv = recv (s_, temp, arg, 0);

		if (rv <= 0)
			return false;

		std::copy(temp, temp + arg, buffer);
	}
}

[THEARTOFWEB]

this seems to be much better:

#include <algorithm>

int Socket::ReceiveBytes(char * buffer, int buffersize)
{
	if(buffersize > 32768)
		buffersize = 32768;

	char temp[32768];
	int byrv = 0;

	while (1)
	{
		u_long arg = 0;

		if (ioctlsocket(s_, FIONREAD, &arg) != 0)
			return -1;

		if (arg == 0)
			return byrv;

		if (arg > buffersize)
			arg = buffersize;

		int rv = recv (s_, temp, arg, 0);

		if (rv <= 0)
			return -1;

		std::copy(temp, temp + rv, buffer);
		byrv += rv;
	}
}

int bytesreceived = Socket::ReceiveBytes(char * buf, int bufsize);

what about that?

[THEARTOFWEB]

this is my last try:

#include <algorithm>

int Socket::ReceiveBytes(char * buffer, int buffersize)
{
	if(buffersize > 32768)
		buffersize = 32768;

	char temp[32768];
	int byrv = 0;

	u_long arg = 0;

	if (ioctlsocket(s_, FIONREAD, &arg) != 0)
		return -1;

	if (arg == 0)
		return byrv;

	if (arg > buffersize)
		arg = buffersize;

	int rv = recv (s_, temp, arg, 0);

	if (rv <= 0)
	{
		return -1;
	} else {
		std::copy(temp, temp + rv, buffer);
	}
}

[THEARTOFWEB]

#include <algorithm>

int Socket::ReceiveBytes(char * buffer, int buffersize)
{
	if(buffersize > 32768)
		buffersize = 32768;

	char temp[32768];

	u_long arg = 0;

	if (ioctlsocket(s_, FIONREAD, &arg) != 0)
		return -1;

	if (arg == 0)
		return 0;

	if (arg > buffersize)
		arg = buffersize;

	int rv = recv (s_, temp, arg, 0);

	if (rv <= 0)
		return -1;

	if (rv == 0)
		return 0;

	if (rv > 0) 
		std::copy(temp, temp + rv, buffer);

}

where FIONREAD:

Determine the amount of data which can be read atomically from socket s. argp points to an unsigned long in which ioctlsocket stores the result. If s is stream oriented (for example, type SOCK_STREAM), FIONREAD returns an amount of data which can be read in a single recv; this may or may not be the same as the total amount of data queued on the socket. If s is message oriented (for example, type SOCK_DGRAM), FIONREAD returns the size of the first datagram (message) queued on the socket.

So I basically I will recv according to ioctlsocket. if the latter is > then buffersize I will read buffersize (arg = buffersize) if it's less...well...be just happy with that...if it's 0 or less don't even bother copying the buffer and return accordingly.

that's all

what about my idea?

[THEARTOFWEB]

I think I got to the solution:

#include <algorithm>

int Socket::ReceiveBytes(char * buffer, int buffersize)
{
	if(buffersize > 32768)
		buffersize = 32768;

	char temp[32768];

	u_long arg = 0;

	if (ioctlsocket(s_, FIONREAD, &arg) != 0)
		return -1;

	if (arg == 0)
		return 0;

	if (arg > buffersize)
		arg = buffersize;

	int rv = recv(s_, temp, arg, 0);

	if (rv < 0)
		return -1;

	if (rv == 0)
		return 0;

	if (rv > 0)
		std::copy(temp, temp + rv, buffer);
	
	return rv;
}

usage:

const int buffsize = 4096;

char buff[buffsize];

while( Socket::ReceiveBytes(buff, buffersize) > 0 )
{
// ...use buff...
}

[hoxsiew]

What are you trying to do?

[THEARTOFWEB]

reading data from teh winsock

[ThachQuanVy]

reading data from teh winsock

reading data from teh winsock

Then,... (expansion of your man purpose is needed)
Just reading your man lengthy if sickkening me

[hoxsiew]

So what happens if you request some number of bytes from the socket and the stack only has some amount less than that? It looks like your routine, as implemented, will return a fragmented buffer and the remaining bytes, when finally received, will be silently dropped.

[THEARTOFWEB]

yeah...this is supposed to be a little better:

int Socket::ReceiveBytes(char * buffer, const int buffersize)
{
	u_long arg = 0;
	if (ioctlsocket(s_, FIONREAD, &arg) != 0)
		return -1;

	if (arg == 0)
		return -1;

	if (arg > (u_long)buffersize)
		arg = buffersize;

	return recv (s_, buffer, arg, 0);
}

[hoxsiew]

I don't see how this is any different. I don't know the nature of your protocol, but you still have the situation where a packet of data less that the size you are expecting will be available on the stack. Using FIONREAD will tell you how how many bytes are in the stack's buffer, but that's not necessarily how many bytes were sent by the machine on the other end of the connection.

[THEARTOFWEB]

well I think winsock does some internal buffer...infact when I use that function to retrieve a binari file from a webserver the return value is often -1 ... the following is how I use the function in my while loop:

	while(1)
	{
		Sleep(100);

		int bsize = s.ReceiveBytes(buffer, 32768);


		if(bsize>0)
		{
			hfile.write(buffer, bsize);
			count++;
		}

		if(count>500)
			break;
	}

All I know is that it's very unlikely for the socket to return more the 32768...usually the winsock bufferssomething like 8192 bytes...

[hoxsiew]

Well, I suppose this would work for transferring a file the way you show, assuming all goes well, but a few things bother me. Why use FIONREAD? You don't seem to care about blocking since your calling this from a while() loop anyway. You use a 100 ms sleep in the main loop which is pretty inefficient if the data are already on the stack.