meta data for this page
  •  

Differences

This shows you the differences between two versions of the page.

Link to this comparison view

Both sides previous revision Previous revision
Next revision
Previous revision
courses:ct30a5000:assignment2notes [2012/10/16 21:29]
julaakko [Test your application!]
courses:ct30a5000:assignment2notes [2012/11/06 20:42] (current)
julaakko [Comments]
Line 8: Line 8:
 | 0312399 | Ok division of code. Partial send should handle the possible error in sending, same for reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Nice wey of tokenizing user input. With IPv6 support you should use struct sockaddr_storage instead of sockaddr_in → IPv6 support is insufficient. Server message handling can be blocked if client sends only 4 bytes → server keeps awaiting for more, consider implementing this completely with select and read what you can then process packets after they are fully received. The length of payload in protocol packets is not properly checked using the length-header-nickname. Invalid binary name. Only IPv6 addresses work with client. If client gives plain “/​connect” without any parameters it gets no response but when given next time with a message server reports that nickname is in use. Server does not seem to handle all premature disconnections since in previously mentioned situation the server replies that nickname is in use. If nickname is too long the message is sent to others. | | 0312399 | Ok division of code. Partial send should handle the possible error in sending, same for reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Nice wey of tokenizing user input. With IPv6 support you should use struct sockaddr_storage instead of sockaddr_in → IPv6 support is insufficient. Server message handling can be blocked if client sends only 4 bytes → server keeps awaiting for more, consider implementing this completely with select and read what you can then process packets after they are fully received. The length of payload in protocol packets is not properly checked using the length-header-nickname. Invalid binary name. Only IPv6 addresses work with client. If client gives plain “/​connect” without any parameters it gets no response but when given next time with a message server reports that nickname is in use. Server does not seem to handle all premature disconnections since in previously mentioned situation the server replies that nickname is in use. If nickname is too long the message is sent to others. |
 | 0316023 | Ok division of code. Partial sends are not handled, remember to check how many bytes have been sent and is there need to send the rest. Partial receiving is not handled – the packet length Is not checked properly, you should always verify that we've received a full packet before processing and if not, receive the rest. The length is utilized well in packet parsing – good. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Client can connect to server when already taken nick is replaced with new – nice. | | 0316023 | Ok division of code. Partial sends are not handled, remember to check how many bytes have been sent and is there need to send the rest. Partial receiving is not handled – the packet length Is not checked properly, you should always verify that we've received a full packet before processing and if not, receive the rest. The length is utilized well in packet parsing – good. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Client can connect to server when already taken nick is replaced with new – nice. |
-| 0342246 | Adding more comments to code wouldn'​t hurt in the future. Good division of code. Nice way of handling partial data. Might be a good idea to differentiate between select() error (-1) and timeout (0). Quitting the application when less than header size is read is not really a good idea and with MSG_WAITALL the application might be vulnerable for DoS attacks – read what you can and if it is not enough try to read more next time data is detected on socket. Otherwise partial data reading/​writing is done well but errors with recv() and send() (-1) should be handled too. The nickname buffer is not padded with NULLs since it is not initialized at any point. Invalid binary name. Duplicate nicknames are not checked. Client can be crashed with invalid input (letters and numbers as port)+| 0342246 | Adding more comments to code wouldn'​t hurt in the future. Good division of code. Nice way of handling partial data. Might be a good idea to differentiate between select() error (-1) and timeout (0). Quitting the application when less than header size is read is not really a good idea and with MSG_WAITALL the application might be vulnerable for DoS attacks – read what you can and if it is not enough try to read more next time data is detected on socket. Otherwise partial data reading/​writing is done well but errors with recv() and send() (-1) should be handled too. The nickname buffer is not padded with NULLs since it is not initialized at any point. Invalid binary name. Duplicate nicknames are not checked. Client can be crashed with invalid input (letters and numbers as port) |
 | 0342327 | Ok division of code but reading some guides about code indentation wouldn'​t hurt – hard to read this stuff. More comments would ease the reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Remember to always check that correct amount of data is sent. Partial sending or receiving is not handled. Do not expect that you have more data awaiting in the network buffer – your approach will open possibilities for easy DoS attacks as server would block when only header is sent.  Lots of valgrind errors. Nickname change from client leaves a newline marker into nickname and server does not initialize the nickname list that is sent, each request increases the list that is sent. Client leaks memory. | | 0342327 | Ok division of code but reading some guides about code indentation wouldn'​t hurt – hard to read this stuff. More comments would ease the reading. For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Remember to always check that correct amount of data is sent. Partial sending or receiving is not handled. Do not expect that you have more data awaiting in the network buffer – your approach will open possibilities for easy DoS attacks as server would block when only header is sent.  Lots of valgrind errors. Nickname change from client leaves a newline marker into nickname and server does not initialize the nickname list that is sent, each request increases the list that is sent. Client leaks memory. |
 | 0358454 | Good division of code.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Partial sending or receiving not handled, the approach using loop to peek the length will introduce problems at the second round since it starts writing to buf[0] at every round. You should always check that is the packet fully received before processing – also processing could be done after the reading of incoming data: separating these functions might bring flexibility. The packet length is converted to network order with htons() instead of htonl() → the result is 16bit instead of 32bit, this works in your application but does not work with others. You should make sure that nicknames have terminating NULL at the end especially when the nickname list is sent (nicks must be separated with NULLs as stated in specification). Client leaks memory. Client does not report connection error if e.g. port is wrong. | | 0358454 | Good division of code.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). Partial sending or receiving not handled, the approach using loop to peek the length will introduce problems at the second round since it starts writing to buf[0] at every round. You should always check that is the packet fully received before processing – also processing could be done after the reading of incoming data: separating these functions might bring flexibility. The packet length is converted to network order with htons() instead of htonl() → the result is 16bit instead of 32bit, this works in your application but does not work with others. You should make sure that nicknames have terminating NULL at the end especially when the nickname list is sent (nicks must be separated with NULLs as stated in specification). Client leaks memory. Client does not report connection error if e.g. port is wrong. |
Line 19: Line 19:
 | 0404544 | Ok division of code but using multiple files would make it easier to read.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). The amount of received data is not checked – you should not expect that you always get enough data. Partial receiving is not handled. Nicknames should be separated with NULLs (\0) instead of semicolon. You should set the data buffers to NULLs before sending since it might contain some garbage – this is because you're not padding the nickname with NULLs before putting it into buffer, look up the memset() function. Lots of Valgrind errors. Duplicate nicknames are not checked. Server does not inform clients when quitting. Lots of leaking memory. | | 0404544 | Ok division of code but using multiple files would make it easier to read.  For possible longer user input it would be good idea to check if fgets() stopped at newline marker (is there anything else in the buffers). The amount of received data is not checked – you should not expect that you always get enough data. Partial receiving is not handled. Nicknames should be separated with NULLs (\0) instead of semicolon. You should set the data buffers to NULLs before sending since it might contain some garbage – this is because you're not padding the nickname with NULLs before putting it into buffer, look up the memset() function. Lots of Valgrind errors. Duplicate nicknames are not checked. Server does not inform clients when quitting. Lots of leaking memory. |
 | 0404557 | Lots of implicit declaration errors, see manual pages of those functions and use the headers mentioned there. Ok division of code but usage of more functions will provide flexibility to code. Partial receiving or sending not handled – you should always check how much has been sent or received. Valgrind errors. Nothing really works with multiple clients, only messages from first client reaches the server. Do not expect that clients send data immediately after connecting, after the client is accepted wait for select to tell if the new socket (not the server listening socket) contains data available for reading. Read what you can in one recv() and check if you got a full packet ready to be processed, if not, read the rest when it arrives and process it later – this would require some buffering of data for each client. You should at least initialize all variables, especially buffers, before use.  | | 0404557 | Lots of implicit declaration errors, see manual pages of those functions and use the headers mentioned there. Ok division of code but usage of more functions will provide flexibility to code. Partial receiving or sending not handled – you should always check how much has been sent or received. Valgrind errors. Nothing really works with multiple clients, only messages from first client reaches the server. Do not expect that clients send data immediately after connecting, after the client is accepted wait for select to tell if the new socket (not the server listening socket) contains data available for reading. Read what you can in one recv() and check if you got a full packet ready to be processed, if not, read the rest when it arrives and process it later – this would require some buffering of data for each client. You should at least initialize all variables, especially buffers, before use.  |
 +
 +===== Making sure that all data is sent and received with TCP =====
 +
 +A example when all is sent in one call, e.g. in client ([[http://​beej.us/​guide/​bgnet/​output/​html/​multipage/​advanced.html#​sendall|another one]]):
 +<code c>
 +int write_to_socket(int sockfd, char* data, unsigned int data_len, unsigned int *data_written)
 +{
 +  int wrote = 0;
 +  ​
 +  // Initialize the pointer value to 0
 +  *data_written = 0;
 + 
 +  // While we have something to write
 +  while(*data_written < data_len)
 +  {
 +    // Write remaining data
 +    if((wrote = send(sockfd,&​data[*data_written],​data_len - *data_written,​0)) < 0) return -1;
 +
 +    // Update the written amount for pointer
 +    *data_written += wrote;
 +  }
 +  ​
 +  return 1;
 +}
 +</​code>​
 +
 +
 +For receiving data you have to use a different approach, especially with multiple connections. If the data is not received in one ''​recv''​ call keep reading until the full packet is completely read. One example:
 +<code c>
 +
 +int read_from_socket(int sockfd,​char* buffer, unsigned int total, unsigned int *remaining)
 +{
 +  do
 +  {
 +    // Read 0 or less -> we're done here, return -1 or 0
 +    if((data_read = recv(sockfd,&​buffer[total-*remaining],​*remaining)) < 1) return data_read;
 +  ​
 +    // otherwise update the remaining amount
 +    else *remaining -= data_read;
 +    ​
 +  } while (*remaining > 0)
 +  ​
 +  return 1; //ok
 +
 +</​code>​
 +
 +Just keep in mind that when designing the reading approach for a server it is good way to balance the load by reading only certain amount of data per client and then checking if it is fully received. If not, proceed to the next client and so on.
 +
 +A {{:​courses:​ct30a5000:​tcptest.tar.gz|demonstration server and client}} using ''​MSG_DONTWAIT''​ flag with ''​send''​. Client sends a 1 Mb of data to the server and both quit after the data is sent full / all is received. Compile with ''​make''​ and run the client with <​code>​./​tcptestc <​serverIP>​ <server port></​code>​ and the server with <​code>​./​tcptests <​port></​code>​
 +
 +The ''​MSG_DONTWAIT''​ demonstrates the effect when the send buffer is full and the application cannot write more -> the function returns and in next round attempts to write more into the buffer. Without the flag the ''​send()''​ function **blocks until all requested data is sent** - it will fill the buffer, wait until there is space, fill the remaining space, wait .. and it continues until all is sent.
 +
 +