Will my winsock receive function be ok new to winsock

Started by
17 comments, last by ankhd 15 years, 5 months ago
Hi there all. I'm now at the stage of receiving socket data TCP over IP. I've read on some web pages that the MSG_PEEK has some problems is this true The window ask say it's ok. So I came up with this. And is this the way I should head some feed back needed. DWORD ReceiveSocketData(LPVOID *out) { //first read in the header (it's just the size of the data char inbuf[100]; Int s = sizeof(int); Int bytesrecv = 0; //use peek to see if we can read in the header bytes loop until we get all header While(bytesrecv < s) { Bytesrecv = recv(socket, inbuf, s, MSG_PEEK); if(bytesrecv ==0 || WSAGetLastError() == WSAECONNRESET) { return 0; } }// end reading header //copy the size from the buffer memcpy(&s, sizeof int, buf, sizeof int); char *msg = new char; while(bytesrecv &lt; s) { bytesrecv = recv(socket, msg, s, 0); //check for errors } *out = msg; return s; }
Advertisement
A quick google has the following to say about it.

Use the same loop as you have, but instead of peeking, just do actual receive in the buffer. If you have non-blocking sockets, check for WSAEWOULDBLOCK to see if there's more data available.
You're copying some size from a buffer "buf" which isn't declared. If you mean "inbuf" then that's a protocol field -- but you're not guaranteed that that amount of data is actually there, nor are you guaranteed that it will ever be there. Someone might connect to your system, and send a number of "100000000 bytes follows" messages, and then send nothing, just to run your system out of memory or make it block forever.

Also, you're not even guaranteed that the entire size has arrived when you call MSG_PEEK -- perhaps only 1 or 2 or 3 bytes have arrived. If you assume you have 4 bytes, then you may get garbage data in that copy.

Finally, returning a raw char[] allocated with new[] is a poor API -- especially since you return it as a void*. I would recommend wrapping the buffer in an object, or letting the user provide a max buffer size, instead. Returning allocated raw pointers is always more trouble than it's worth (from a memory leak perspective, as well as from an API perspective when you start putting functions in DLLs).
enum Bool { True, False, FileNotFound };
I understood the guy needs to send an integer over a net. This may not work even all is fine with network. Due to different memory layouts, endianess, routers and etc. you may receive damaged data. There are two ways to do that - 1) use machine to network byte-order conversion funstions (like DNS protocol does) or 2) convet data into a string - send it over a network - convert a string into a number (like HTTP protocol does).
Yes it was a error buf is ment to be inbuf I only have a iPhone for my web. And can't do scroll boxes.
Any how. Your saying that if I do this
while(bytesrecv < sizeof int)
{
bytesrecv += recv( socket, &inbuf[ bytesrecv], sizeof char, 0);
//do error
}

// should have sizeof int bytes
//read the rest of the bits here 1 char at a time until we get to error or the max size.
The problem with that loop is that someone can connect, send a single byte, and then send nothing, and your server will go into an infinite loop. If the socket is non-blocking, the infinite loop will consume CPU; else it will just freeze up the server blocking inside recv().
enum Bool { True, False, FileNotFound };
Should I put a timeout value in the loop.
Does any one have away to do it. all the tutorials I've read check for
Socket errors for the loops condition would that also lock up on bogus data.
Should I send just one byte for maybe a type and not the size. This way I can look up the type to get the data size.
One last question should I be using sizeof or just hard code say 4bytes for the flag or size member.
I'm totally lost now. Maybe I should just keep using directplay does every thing I need and I've used it twice now. But they are going to remove it from the sdk if not all ready.
You must buffer the data locally rather than just reading fixed-length items from the network. Pull everything from the socket into your buffer, then scan the buffer for your object (in your example, an int). If you find a whole int, take it out of the buffer. If you don't, add more to the buffer next time and repeat the process.

EDIT: in answer to your other questions:
You can send the type or the size, or if needed both. Whatever is easiest for you. Just be careful to check when you read a size value because you don't want some hacker sending your server a message claiming to send you a 2 gigabyte package.

Use sizeof when you know the object you're sending will be that size - simple, really. If the object can be of variable size then you have to treat it differently anyway. The only downside of using sizeof is that 2 platforms might have different sizes for the same named variable. You might want to use something like the Boost cstdint header to get types of a fixed size.
If you want a higher-level library than sockets, there are several mentioned in the Forum FAQ.
enum Bool { True, False, FileNotFound };
what do you call a deer with no eye's?????
No eye deer. that me I still don't get it. Could some one please post example.
This is what I get from Kyloton correct me if I'm wrong.

Read socket data in to a buffer eg vector<char> inbuff.
Then read data from wsocks receive until ( when ? how much data should I fetch with receive)
Receive returns now my app can check the inbuffer(? but how do I handle partial data fetches when the buffer is full. Ans return and when I fetch more do it on the next round of input).

Meanwhile some where in the app handlesocketdata can. Check the type get the size then copy the serialized data to it's class type.

Don't know what to do about a hackers sending bogus data. I guess I will crash.

This topic is closed to new replies.

Advertisement