C244a Programming Projects

Coding Guidelines


To goal of the programming assignments in CS244a is to teach you how to write high quality networking applications, and how the network infrastructure works. It is an important part of the class that you learn how to write high quality, large complex programs; and so we encourage (in fact, we insist!) that you practice good software design and implementation. You will probably have gained some experience in this already in the pre-requisite classes such as the CS106 series and CS140. However, due to the size and complexity of the assignments in CS244a, you can expect to find it difficult to well-structured and well-commented programs.

In previous years, many cs244a students have lost more points on style and design than they lost on the actual functionality of the program. On the other hand the best submission have consistenly blown us away with their crystal clear design and code that reads like Shakespeare. The idea of this page is to show you some of the best and worst examples we have come across in the past. We hope you will learn from others, and so become better network programmers.

Below are a number of topics should help you in writing better software for this class, and also for the future. Both the good and the bad code examples below are only slightly modified from real assignment submissions.

Design Basics and Coding Style

If you have not taken a Stanford programming class (106A/B/X), please read their handout CS106X - Developing Good Style. It covers a lot of the basics including commenting, overcommenting, choosing identifiers, formatting, expression handling and decomposition.

Next you should probably have a look at the Grading Guidelines for the CS244a assignments. They contain a section on what the TA's will deduct points for.

How exactly you format your code and how you pick variable names is up to you. Useful reading might be the Linux Kernel Coding Style. This is how the low-level networking software that runs most servers in the internet is written, it might work for you as well.

One issue mentioned in the above guides that we want to emphasize is the length of functions. A good function is between 5 and 50 lines. If your function is longer, you should split it up into a few subroutines. Long functions will cost you style points!

Global Variables

There is absolutely no good reason to have global variables in any of the assignments. Global variable makes sense in some very limited scenarios, but not for any of the programs you have to develop for this class.

For example, consider the following ftp client that keep all data attributed to a session as global variables:

/**
* global constants
**/
char HostName[MAX_HOSTNAME_LEN];
ResultSetElement servResultSet[MAX_RESULT_SET];
short servResultSetNum;
char UserName[MAX_USER_NAME_PASS];
char Pass[MAX_USER_NAME_PASS];
int portnumber;
int TotalBytesCopied;
int depth;

The above example might function correctly. However, imagine you later wanted to implement multiple parallel sessions. You would have to go through all of your code and replace the global variables with session specific variables. A better way to implement it might be to have the session specific variables in a structure that is passed along. For example:

/* copyInfo transaction - various data, mostly from cmd line
* needed during the processing of the copy transaction
*/
struct copy_trans_t {
char *servHostName; /* source server name */
char *usetName; /* username for logging into the machine */
char *passwd; /* password for authentication */
short servPortNumber; /* port number of ftp, stored in netbyte */
char *fileExt; /* extension of files to retrieve */
int numDirLevels; /* number of directory levels to traverse */
char *servDir; /* start directory on server */
char *lclDir; /* start directory on local machine */
};

[...]

void ParseCommandLine(struct copy_trans_t *copyInfo, int argc, char **argv)
{

This structure can now be passed to all functions that make use of this information. If you ever wanted to have several sessions in parallel each would get its own structure and no changes to the functions would be required.This example is also well commented and allows strings of any length (as opposed to fixed length strings).

Decomposition

When you are using very similar code in several functions, you are probably doing something wrong. Try to find an abstraction for the common functionality and put it into a seperate function. For example in the code below, both functions have almost exactly the same code.

/*--------------------------------------------
* GetHostDir
*--------------------------------------------
* pre : takes the socket, serverHostName, extension, localdir
* post: if fails, frees the serverHostName, extension, localdir
* otherwise, gets host directory and returns the dir on heap
*/
char * GetHostDir(int s, char *serverHostName, char *extension, char *localdir)
{
char * serverReply;
if(strlen(PWDSTR) != write(s, PWDSTR, strlen(PWDSTR)) ){
fprintf(stderr, "ERROR: failed while writing to server\n");
close( s );
free(serverHostName);
free(extension);
free(localdir);
exit( FAILURE );
}
serverReply = getReplyFromServer(s);
/* fprintf(stderr,"%s", serverReply); */
return serverReply;
}

/*---------------------------------------------
* LogOffServer
*---------------------------------------------
* pre : takes the socket, serverHostName, extension, localdir
* post: if fails, frees the serverHostName, extension, localdir
* otherwise, sends a quit message to the host/server
*/
void LogOffServer(int s, char *serverHostName, char *extension, char *localdir)
{
char * serverReply;
if(strlen(QUITSTR) != write(s, QUITSTR, strlen(QUITSTR)) ){
fprintf(stderr, "ERROR: failed while writing to server\n");
close( s );
free(serverHostName);
free(extension);
free(localdir);
exit( FAILURE );
}
serverReply = getReplyFromServer(s);
/*fprintf(stderr,"%s", serverReply);*/
free(serverReply);
}

[...]

A better way would have been to encapsulate the writing of the command and the error handling in one function that takes the command as a parameter. The above code has a few other issues. It will abort if only a part of the string were written to the socket. The write() call will sometimes not write all characters in one pass but instead return with a partial write. Also some of the parameters that are passed are never used.

Readability

Below is an example of how decomposition can make a program much more readable. The function below sends an ARP reply packet. This function in most implementations is pretty hard to read as it contains a lot of code that just assigns the values to the different fields in the packet, reorders the bytes into network byte order etc. This implementation here however is very readable. (from a solution of Neil Daswani)

/*---------------------------------------------------------------------
* Method: sendARPReply
* Scope: Global
*
* Takes as input an ARP request header and a destination hardware
* address (dstHwdAddr). Responds to the request by sending an
* ARP Reply through sr's interface iface.
*
*---------------------------------------------------------------------*/

void sendARPReply (struct sr_instance *sr,
const char *iface,
struct sr_arphdr *requestHdr,
char *dstHwdAddr) {

char *reply_hdr = 0;
int reply_hdr_len= 0;

char *reply_ether_frame = 0;
int reply_ether_len = 0;

/* create the reply */

createARPHdr (ARP_REPLY,
dstHwdAddr, /* src hwd addr */
requestHdr->ar_tip, /* src ip */
requestHdr->ar_sha,
requestHdr->ar_sip,
&reply_hdr,
&reply_hdr_len);

createEthernetPacket (requestHdr->ar_sha,
dstHwdAddr,
ETHERTYPE_ARP,
reply_hdr,
reply_hdr_len,
&reply_ether_frame,
&reply_ether_len);

dumpPacket (reply_ether_frame,reply_ether_len);

/* send the reply */

sr_send_packet(sr,
reply_ether_frame,
reply_ether_len,
iface);
}

The reason it is so clean is that all of the packet assembly has been moved to the createARPHdr and createEthernetPacket functions that are in a seperate packet assembly library that the student wrote. This library is then used by several functions to manipulate packets. This decomposition makes the code readable and debugging much easier.

Error Handling

Error handling is critical for any networking code; errors happen, sockets get terminated unexpectedly, and so it is important to recover gracefully and cleanly when things go wrong.  Often, students write error handlers that clutter their code. It doesn't need to. Below is a bad example of code that logs into an ftp server. Most of the code is actually about handling errors and reading the results from the server. The error handling will not even work in all cases. A partial write() will cause the program to abort.

   testString = "USER anonymous\r\n";
printf(testString);
if(strlen(testString) != write(sockfd, testString, strlen(testString))) {
fprintf(stderr, "ERROR: failed while writing to server\n");
close(sockfd);
exit(FAILURE);
}

/* read server's response */
while(1) {
tmpbuf[1] = '\0';
i = 0;
while(1) {
if((bytes_read = read(sockfd, tmpbuf, 1)) == 0) { /* read pipe byte by byte to determine where the NVT ASCII line ends */
break;
} else if(bytes_read > 0) {
replyBuffer[i] = tmpbuf[0];
if(replyBuffer[i] == '\n' && replyBuffer[i - 1] == '\r') { /* watch for the line terminator */
replyBuffer[i + 1] = '\0';
break;
}
i++;
}
}

printf(replyBuffer);
if(replyBuffer[3] == '-') {
continue;
} else {
break;
}
}

testString = "PASS anon@anonymous.com\r\n";
printf(testString);
if(strlen(testString) != write(sockfd, testString, strlen(testString))) {
fprintf(stderr, "ERROR: failed while writing to server\n");
close(sockfd);
exit(FAILURE);
}

The next code segment has the same functionality but only needs 5 lines. This is mainly due to elegant error handling. Note that this example uses 'goto' which usually should not be used in C. However in this case it emulates the throw/catch model of C++ or Java to produce easily readable code. (from a solution by Kenneth Ashcraft)

/* CHECK_ERR
* ---------
* When the boolean 'test' is true, we jump to 'label'.
*/
#define CHECK_ERR(test, label) \
if ( (test) ) { \
goto label; \
}

/* PRINT_ERR
* ---------
* When the boolean 'test' is true, we print 'msg' to stderr
* and jump to 'label'.
*/
#define PRINT_ERR(test, label, msg) \
if ( (test) ) { \
fprintf(stderr, (msg)); \
goto label; \
}

[...]

/* Function: transfer_files
* ------------------------
* Logs into the server, sets the correct modes, and processes the
* src_dir.
*/
static int transfer_files(struct sock_holder *s, char *ext,
int directory_depth, char *dst_dir, char *src_dir)
{
unsigned int bytes_recvd;
char dst_path[PATH_MAX];
char src_path[PATH_MAX];
int ret;

PRINT_ERR(send_cmd(s, "USER", "anonymous") < 0,
fail, "ERROR: could not send USER command\n");
CHECK_ERR(wait_for_code(s, 331) < 0, fail);

CHECK_ERR(send_password(s) < 0, fail);
CHECK_ERR(wait_for_code(s, 230) < 0, fail);

PRINT_ERR(send_cmd(s, "MODE", "S") < 0,
fail, "ERROR: Could not set mode\n");
CHECK_ERR(wait_for_code(s, 200) < 0, fail);

PRINT_ERR(send_cmd(s, "STRU", "F") < 0,
fail, "ERROR: Could not set structure\n");
CHECK_ERR(wait_for_code(s, 200) < 0, fail);

PRINT_ERR(send_cmd(s, "CWD", src_dir) < 0, fail,
"ERROR: Could not send CWD command\n");
[...]

/* The user may have specified a relative dir. Need to get the
* absolute path */
CHECK_ERR(get_real_remote_dir(s, src_path, PATH_MAX), fail);

PRINT_ERR(chdir(dst_dir) < 0, fail,
"ERROR: Could not cd to dst_dir\n");
PRINT_ERR(getcwd(dst_path, PATH_MAX) == NULL, fail,
"ERROR: Could not find real path\n");

[...Main Loop...]

return 0;

fail:
return -1;
}

#define vs. enum

Using defines instead of values is often a good idea. If the values are local to your program, using enum is often a better idea. In the next example, enum is used to identify states clearly and unambiguously for a TCP stack in Assignment #2. (code from Martin Casado)

/*----------------------------------------------------------------------
* Struct STCP STATES
*---------------------------------------------------------------------*/


enum {
SYN_ACT_SENT,/* active connection sent the
first SYN packet and is awaiting
a SYN-ACK response */
SYN_PSV_INIT,/* passive connection is waiting an
initial SYN */
SYN_PSV_SENT,/* passive connection sent SYN-ACK but
has not gotten any user data from
client so SYN-ACK may have been lost.
If we receive a SYN packet in this state,
must SYN-ACK again!
*/

CSTATE_ESTABLISHED, /* connection has been established
for sure, any SYN or SYN/ACKs in
this state are an error */

FIN_ACT_SENT,/* active side of connection teardown
sent first FIN packet and waiting for
FIN response */
};

Using State Machines

Network Protocols can often be described using finite state automata. Below is a great example of how to use the state machine of STCP from Assignment #2 to structure the program.

/**********************************************************************/
/* transport_sock_io
*
* Called in the child process
*
*/
static void transport_sock_io(context_t *ctx)
{
[...]

header = (STCPHeader *)buffer;
len = network_recv(ctx->sockfd, buffer, sizeof(buffer));
[...]

switch(ctx->connection_state)
{
case CSTATE_CLOSED:
switch (recv_flags) {
case (TH_SYN):
case (TH_SYN | TH_ACK):
case (TH_FIN):
case (TH_FIN | TH_ACK):
case (TH_ACK):
default: /* data */
break;
}
break;

case CSTATE_SYN_WAITING: /* waiting SYN */
switch (recv_flags) {
case (TH_SYN):
[...do stuff...]
break;
case (TH_SYN | TH_ACK):
case (TH_FIN):
case (TH_FIN | TH_ACK):
case (TH_ACK):
default: /* data */
break;
}

break;

case CSTATE_SYN_SENT: /* SYN is sent, waiting for SYN_ACK */
switch (recv_flags) {
case (TH_SYN):
case (TH_SYN | TH_ACK):
[...do stuff...]
break;
case (TH_FIN):
case (TH_FIN | TH_ACK):
case (TH_ACK):
default: /* data */
break;
}
break;

case CSTATE_ESTABLISHED: /* connection is established */
switch (recv_flags) {
case (TH_SYN):
case (TH_SYN | TH_ACK):
case (TH_FIN | TH_ACK):
break;
case (TH_FIN):
[... do stuff...]
break;
case (TH_ACK):
[... do stuff...]
break;
default:
/* data */
[... do stuff...]
}
break;

case CSTATE_ACK_WAITING: /* waiting ACKs for sending FIN */
switch (recv_flags) {

case (TH_SYN):
case (TH_SYN | TH_ACK):
case (TH_FIN):
case (TH_FIN | TH_ACK):
[...do stuff...]
exit_control_loop(1);
break;


break;

case (TH_ACK):
[...do stuff...]
break;

default: /* data */
break;
}

case CSTATE_FIN_ACK_WAITING: /* waiting FIN_ACK */

switch (recv_flags) {

case (TH_SYN):
case (TH_SYN | TH_ACK):
case (TH_ACK):
break;
case (TH_FIN):
[...do stuff...]
exit_control_loop(1);
break;

case (TH_FIN | TH_ACK):
[...do stuff...]
break;

default: /* data */
break;
}
break;
}
}


[ STANFORD UNIVERSITY ]