Reputation: 358
I tried to make a file downloader using Winsock but I realized that if the Internet connection is slower the client receives some unwanted data along with the data sent by the server.
So I made a simple test:
From the server I am sending the numbers from 1 to 30000:
char* buf = (char*)malloc(BUFLEN);
for( int i = 0;i < 30000;++i ){
ZeroMemory(buf, BUFLEN);
itoa(i+1, buf, 10);
send(current_client, buf, BUFLEN, 0);
}
free(buf);
The client receives and saves them:
char *buf = (char*)malloc(DEFAULT_BUFLEN);
ofstream out;
out.open(filename, ios::binary);
for( int i = 0;i < 30000;++i ){
ZeroMemory(buf, DEFAULT_BUFLEN);
recv(ConnectSocket, buf, DEFAULT_BUFLEN, 0);
out.write(buf, DEFAULT_BUFLEN);
out << endl;
}
out.close();
free(buf);
And I expect in the file to see something like that:
1
2
3
4
...
30000
But instead there are some extra packets, containing '/0', transmitted and the file looks like that:
1
2
3
4
5
6
...
2600
If I try to skip the '/0' packets, data from the server is also skipped like that:
1
2
3
4
5
6
9 <- 7 and 8 are missing
...
2600
What am I doing wrong ?
Upvotes: 2
Views: 2017
Reputation: 358
I figured out what was the problem.
Exactly as Joachim and Remy said sometimes the client doesn't receive the entire buffer so I modified the code a little:
from the server side:
sent = 0;
while( sent != BUFLEN ){
sent += send(current_client, (buf+sent), BUFLEN-sent, 0);
}
and client:
recvd = 0;
while( recvd != DEFAULT_BUFLEN ){
recvd += recv(ConnectSocket, (buf+recvd), DEFAULT_BUFLEN-recvd, 0);
}
and I know that the buffer is not filled entirely, normally I'm sending binary data as char* , not just integers.
Upvotes: -1
Reputation: 598134
recv()
tells you how many bytes were actually received. You are ignoring that value that outputting the entire buffer even if it was not filled up completely. Same thing in the server end - you are sending more data then you are formatting.
You are also not taking into account that both send()
and recv()
can send/receive fewer bytes then you requested.
Try this instead:
bool sendraw(SOCKET socket, void *buf, int buflen)
{
unsigned char *p = (unsigned char*) buf;
while (buflen > 0)
{
int sent = send(socket, p, buflen, 0);
if (sent < 1) return false;
p += sent;
buflen -= sent;
}
return true;
}
for( int i = 0; i < 30000;++i )
{
int j = htonl(i+1);
if (!sendraw(current_client, &j, sizeof(int)))
break;
}
.
bool recvraw(SOCKET socket, void *buf, int buflen)
{
unsigned char *p = (unsigned char*) buf;
while (buflen > 0)
{
int received = recv(socket, p, buflen, 0);
if (received < 1) return false;
p += received;
buflen -= received;
}
return true;
}
ofstream out;
out.open(filename, ios::binary);
for( int i = 0; i < 30000;++i )
{
int j;
if (!recvraw(ConnectSocket, &j, sizeof(int)))
break;
out << ntohl(j) << endl;
}
out.close();
Upvotes: 3
Reputation: 409472
From the manual page of recv
:
For connection-oriented sockets (type SOCK_STREAM for example), calling recv will return as much data as is currently available—up to the size of the buffer specified.
This means that recv
might give you incomplete packets. If you have a fixed length or a terminator on the packets, you need to call recv
in a loop appending to the buffer until all is received. However this presents another problem, that the second call to recv
might give you the remaining of the last packet, but also parts of another packet.
In your case it might also be that the sender is sending faster than the receiver can receive them, in which case recv
will either block (if socket is blocking) or return an error (if socket is non-blocking). You have to check the value returned from recv
to know what it is. If the socket is non-blocking, then recv
will return SOCKET_ERROR
(i.e. -1
) and WSAGetLastError
will return WSAEWOULDBLOCK
.
Upvotes: 3