Assignment3: Notes

These pages will contain notes about the third assignment and correct answers for questions.

When using work of others (copying)

In this assignment a demonstration code was provided and it was ok to directly copy it. However, remember that when copying others code it is classified as plagiarism if the source is not stated anywhere, or the code is copied against its license. Generally it is not recommended to directly copy others work (nor it is allowed on this course!) but if you do (e.g. a company has some code base for most generic stuff) GO THROUGH THE CODE AND THINK WHAT IT DOES BEFORE COPYING IT INTO YOUR APPLICATION!

General comments

Here are some generic comments about the returns.

Protocol/Address family

Generally with servers PF_INET6 should be used to accept both IPv4 and IPv6 connections but on client it is useful to use PF_UNSPEC and let the client application select appropriate one – otherwise IPv4 address resolution usually fails (no minus for this).

fgets()

With fgets() you can specify how many bytes are read and it will not exceed this limit, however, if the given text is longer than the value you specify, it will return only that amount. When reading from standard input it will keep the rest that was read in the buffers. Remember that it reads until newline marker is reached or limit-1 bytes are read (or EOF is detected).

#define BSIZE 512
...
char buffer[BSIZE] = {0};
if(fgets(buffer, BSIZE, stdin) != NULL) {
  if (strlen(buffer) > BSIZE)) printf("this is never reached!\n");
  else printf("If there is a newline at the end, there is nothing more in the buffers (all read)\n"); 
}

You should check if there is a newline at the end (and the length is BSIZE-1 as in example), if there is not you can read the rest and concatenate the result for instance. This is because the select() will not detect the data in the stdio buffers (out of POSIX) and you'll have to check for it.

recvfrom() behavior

Note that if you have incoming more data than you can receive in your buffer the recvfrom() function will discard the rest. With basic text protocols like here this can be used to limit the incoming data amount to the server (e.g. easy way to limit aphorism length). But in other cases this might result in data loss. This is the reason why there is a usage for length variable in UDP protocol packets, too. You cannot recover that remaining data but you can at least check if we got all.

And also, there is no partial receiving/sending with UDP. It has strict packet boundaries. One write/read = one protocol packet.

Random numbers

It would be good idea to initialize the random number generator with some unique seed, e.g. current time in milliseconds. See the rand()/srand() manual page. This is because it is a pseudorandom number generator and with seed the sequence of numbers will be more random.

TEST THE COMMITTED

Remember to always test the committed version. You can always update it (before deadline) or even put it in small pieces into the SVN. A good guideline would be to checkout the assignment into some other directory and test if it compiles and runs.

Specific comments

Student number Comments
0312399 Good division of code. You could use NI_MAXHOST instead of INET6_ADDSTRLEN as latter defines the maximum length only for the IP address, not hostname, see the size differences yourself. You cannot & do not need to use partial sending with UDP since the sendto() will either return error (-1) or block until the data has been sent, check the manual page. If the client cannot resolve address it crashes. Valgrind gives some of errors. See the comment about protocol/address family usage. The server freezes if the database is empty. The sender info should not be sent along aphorism as it might increase the length to over the limit – the aphorisms are limited to 509 characters including the terminating NULL. Server and client both allow oversized aphorisms – if the server selects this oversized aphorism for sending the client freezes and cannot provide output, however the aphorism is saved as correct sized. After 10th aphorism the server still responds that aphorism is added and time is set to 01.01.1970 02:00:00. v No timeout for client for checking server presence.
0316023 Good division of code. See the comment about fgets(). The rand() function would create unique sequence if a seed, e.g. current time in milliseconds is given since it creates only pseudorandom numbers. Server does not check for command length, e.g. “GETaa” would work (from netcat).
0342246 Good division of code. Nice storage for aphorisms with dynamic allocation system. Alternative way to handle timeout with socket option SO_RCVTIMEO – works ok here. See the comment about fgets(). Server does not return usage instructions as error message if invalid command is sent. Server leaks some memory.
0342327 Ok division of code. The rand() function would create unique sequence if a seed, e.g. current time in milliseconds is given since it creates only pseudorandom numbers. Read the comment about fgets(). Use NI_MAXHOST for full length hostnames, INET6_ADDSTRLEN for max IP length, the 16 characters is too little when storing aphorism sender. Remember to initialize the static variables you use in order to reduce odd behavior, in most cases valgrind will help with solving these. Aphorisms should be 509 characters at max, the whole protocol message is 512 – to avoid typing errors use #define to specify the lengths in one place. Server allows too short aphorisms. Valgrind provides errors on client. Client leaks some memory.
0358454 Ok division of code. You could use NI_MAXHOST instead of INET6_ADDSTRLEN as latter defines the maximum length only for the IP address, not hostname, see the size differences yourself. Your application limits the usage of IP addresses only, see what the NI_NUMERICHOST actually means. Read the comment about fgets(). The rand() function would create unique sequence if a seed, e.g. current time in milliseconds is given since it creates only pseudorandom numbers. Nice way of using resend to check if the server exists. Remember to initialize static variables – using valgrind would tell about these. Excess data with AOK message, all commands should be strictly 3 bytes, no NULL (or dot !) at the end required. With long aphorisms the message is ended with APH, might be a buffer initialization problem. For address resolving use getnameinfo() instead of old inet_ntop().
0377037 Good division of code. Sending function(s) are a bit odd, why would they report error when getnameinfo() fails after the actual data has been sent? The calculation of incoming aphorism length Is faulty, maximum is 506 not 509 as specified. Only IPv4 addresses work on client in numeric form, see what the NI_NUMERICHOST actually means. See the comment about protocol/address family usage. Does not report usage instructions when invalid command is received. Server leaks memory. Some aphorisms are saved on the same line in the database and these do not seem to be delivered to client.
0378227 Do these pages look familiar http://bytes.com/topic/c-sharp/answers/278898-maximum-datagram-size-udp-sockets http://wiki.wireshark.org/PacketLoss http://www.informatica.com/downloads/1568_high_perf_messaging_wp/Topics-in-High-Performance-Messaging.htm#UDP-BUFFER-SIZING ? Since these are answers to additional questions, only 5 point penalty is rewarded in this assignment, do this again and the course is over. You could have at least credited the partner (or even better: asked for permission to use this) you originally did this with in 2010 – your ship sails in very murky waters right now, this is accepted now, not ok, but accepted. Invalid binary name and there should be two executables, did you even bother to go through the CHANGED description? There is no IPv6 support. Too short aphorisms can be added. The server should report the server command instructions, not the instructions to run the client (and the binary name is wrong here). The server does not save the sender or date of aphorisms. Server does not shut down nicely.
0404450 Write the answers to additional questions in your own words, no penalty since the source is mentioned and you state that this is not your writing – cannot give points for this though. Good division of code. See the comment about protocol/address family usage. The server can be connected only through localhost address since you've forgotten to give AI_PASSIVE as hints to getaddrinfo() → -2. Sometimes the server does not reply to GET. Server does not reply with usage instructions if command is invalid. Server adds some weird characters at the end, although these have not been sent (with netcat) and it seems that these aphorisms, which have weird characters cannot be sent to client. Aphorism input on client is somewhat faulty, longer aphorisms are not sent to your server (error is reported, also with valgrind.
0404489 Good division of code. You should not expect multipart messages with UDP as it has strict message boundaries, you send the protocol message as a whole (no partial sending, one sendto = one protocol packet) and receive it as a whole. See the note about recvfrom() behavior, sendto() will either return error (-1) or block until the data has been sent. Read the comment about fgets(). Some aphorisms are delivered with weird characters at the end, remember to initialize/zero all buffers. Server creates some valgrind errors when adding the first aphorism.
0404531 Does not compile, needs aphorismserver.c
0404544 Does not compile: aphorismclient.c: In function ‘client’: aphorismclient.c:115:19: error: expected expression before ‘)’ token aphorismclient.c:115:19: error: too few arguments to function ‘udp_send’ In file included from aphorismclient.c:3:0: aphorismudp.h:161:5: note: declared here
0404557 Warnings: aphorismserver.c:54:3: warning: passing argument 2 of ‘bind’ from incompatible pointer type [enabled by default] In file included from /usr/include/netinet/in.h:25:0, from aphorismserver.c:10: /usr/include/x86_64-linux-gnu/sys/socket.h:115:12: note: expected ‘const struct sockaddr *’ but argument is of type ‘struct sockaddr_in *’ aphorismserver.c:99:12: warning: comparison between pointer and integer [enabled by default] aphorismserver.c:134:28: warning: assignment makes integer from pointer without a cast [enabled by default] – you should react to these. You should not expect multipart messages with UDP as it has strict message boundaries, you send the protocol message as a whole (no partial sending, one sendto = one protocol packet) and receive it as a whole. Divide the code into functions – it increases reusability and makes the code more easier to debug. It would be better to read the content of the file into memory when the server starts – reading the file is slow and if the aphorisms are read from file at each request it creates unnecessary io-calls. As the sequence diagram suggests you should pick a random aphorism. You should not expect that if the command is not GET then the command must be ADD, always check the data for validity and in this case, length. You really cannot do the server this way, remember that each of the functions (fgets() for user input and sendto()/recvfrom()/send()/recv() for network sockets) you use are blocking so they will stop your application progression until they receive something. Here the server requires user interaction in order to proceed to the section where the reading from socket is done. Testing is just impossible.“