add some comments

Author: seckcoder

proxylab-handout/Makefile

@@ -40,4 +40,4 @@ handin:
 	(make clean; cd ..; tar cvf proxylab-handin.tar proxylab-handout --exclude tiny --exclude nop-server.py --exclude proxy --exclude driver.sh --exclude port-for-user.pl --exclude free-port.sh --exclude ".*")
 
 clean:
-	rm -f *~ *.o proxy core *.tar *.zip *.gzip *.bzip *.gz
+	rm -f *~ *.o proxy core *.tar *.zip *.gzip *.bzip *.gz test

proxylab-handout/cache.c

@@ -1,10 +1,18 @@
+/*
+ * cache.c - lru cache
+ */
+
 #include "cache.h"
 
+/* return the cache size of the node */
 #define node_cache_size(pnode) (((pnode)->value_len)*sizeof(char))
 
+/* create_empty_node */
 static lru_cache_node_t *create_empty_node()
 {
-    lru_cache_node_t *pnode = (lru_cache_node_t*)Malloc(sizeof(lru_cache_node_t));
+    lru_cache_node_t *pnode = (lru_cache_node_t*)malloc(
+        sizeof(lru_cache_node_t));
+    if (pnode == NULL) return NULL;
     pnode->value_len = 0;
     pnode->key[0] = '\0';
     pnode->value = NULL;
@@ -13,11 +21,14 @@ static lru_cache_node_t *create_empty_node()
     return pnode;
 }
 
+/* free_node */
 static void free_node(lru_cache_node_t *pnode)
 {
     free(pnode->value);
     free(pnode);
 }
+
+/* create_node_from  -- create a node from (key, value) pair */
 static lru_cache_node_t *create_node_from(
         const char *key,
         const char *value,
@@ -34,6 +45,7 @@ static lru_cache_node_t *create_node_from(
     return pnode;
 }
 
+/* lru_cache_init  -- init lru cache */
 void lru_cache_init(lru_cache_t *pcache, size_t max_cache_size)
 {
     pcache->max_cache_size = max_cache_size;
@@ -42,6 +54,7 @@ void lru_cache_init(lru_cache_t *pcache, size_t max_cache_size)
     pcache->sentinel->prev = pcache->sentinel;
 }
 
+/* lru_cache_free  -- free the lru cache */
 void lru_cache_free(lru_cache_t *pcache)
 {
     lru_cache_node_t *cur, *next = NULL;
@@ -55,6 +68,7 @@ void lru_cache_free(lru_cache_t *pcache)
 }
 
 
+/* lru_cache_insert_next  -- insert a node to the list */
 static inline void lru_cache_insert_next(lru_cache_t *pcache,
         lru_cache_node_t *prev,
         lru_cache_node_t *pnode)
@@ -67,6 +81,7 @@ static inline void lru_cache_insert_next(lru_cache_t *pcache,
     pcache->cache_size += node_cache_size(pnode);
 }
 
+/* lru_cache_remove  -- remove the node from the list */
 static inline void lru_cache_remove(lru_cache_t *pcache,
         lru_cache_node_t *cur)
 {
@@ -76,6 +91,8 @@ static inline void lru_cache_remove(lru_cache_t *pcache,
 }
 
 /*
+ * lru_cache_raise  -- raise the position of the node as the most
+ * recently visited node
  * assert prev->next != NULL
  */
 static void lru_cache_raise(lru_cache_t *pcache, lru_cache_node_t *cur)
@@ -85,19 +102,22 @@ static void lru_cache_raise(lru_cache_t *pcache, lru_cache_node_t *cur)
     lru_cache_insert_next(pcache, pcache->sentinel, cur);
 }
 
+/* lru_cache_find  -- find a node according to the key */
 lru_cache_node_t *lru_cache_find(lru_cache_t *pcache, const char *key)
 {
     lru_cache_node_t *cur;
     for (cur = pcache->sentinel->next;
             cur != pcache->sentinel; cur = cur->next) {
         if (strcasecmp(cur->key, key) == 0) {
+            /* If a node is found, it's visited once */
             lru_cache_raise(pcache, cur);
             return cur;
         }
     }
     return NULL;
 }
 
+/* lru_cache_insert  -- insert a new node to the lru cache */
 void lru_cache_insert(lru_cache_t *pcache,
         const char *key,
         const char *value,
@@ -106,6 +126,10 @@ void lru_cache_insert(lru_cache_t *pcache,
     lru_cache_node_t *pnode = create_node_from(key, value, value_len);
     lru_cache_insert_next(pcache, pcache->sentinel, pnode);
     while (pcache->cache_size > pcache->max_cache_size) {
-        lru_cache_remove(pcache, lru_cache_tail(pcache));
+        /* remove the least recently used node */
+        lru_cache_node_t *node_to_remove = lru_cache_tail(pcache);
+        lru_cache_remove(pcache, node_to_remove);
+        /* free the removed node */
+        free_node(node_to_remove);
     }
 }

proxylab-handout/cache.h

@@ -1,31 +1,40 @@
+/*
+ * cache.h - lru cache
+ */
+
 #ifndef __CACHE_H__
 #define __CACHE_H__
 
 #include "csapp.h"
 
 typedef struct lru_cache_node_t {
-    char *value;
-    size_t value_len;
+    char *value;   /* cache content. This is not a null terminated string */
+    size_t value_len;  /* size of the cache */
     // for simplicity, assume key is a null terminated string
-    char key[MAXLINE];
-    struct lru_cache_node_t *next;
-    struct lru_cache_node_t *prev;
+    char key[MAXLINE];  /* key - value */
+    struct lru_cache_node_t *next; /* next node */
+    struct lru_cache_node_t *prev; /* prev node */
 } lru_cache_node_t;
 
+/* lru cache is implemented as a bidirectional list */
 typedef struct lru_cache_t {
-    size_t max_cache_size;
-    lru_cache_node_t *sentinel;
+    size_t max_cache_size; /* maximum size allowed for the cache.
+                              If this is exceeded,
+                              we simply evict the least recently used block */
+    lru_cache_node_t *sentinel;  /* sentinel node is a fake node */
     lru_cache_node_t *front; /* front points to most recently used */
     // lru_cache_node_t *tail;  /* tail points to least recently used */
-    size_t cache_size;
+    size_t cache_size;  /* the size of the cache */
 } lru_cache_t;
 
+/* lru cache operations */
 void lru_cache_init(lru_cache_t *pcache, size_t max_cache_size);
 void lru_cache_free(lru_cache_t *pcache);
 
 lru_cache_node_t *lru_cache_find(lru_cache_t *pcache, const char *key);
 
-void lru_cache_insert(lru_cache_t *pcache, const char *key, const char *value, size_t value_len);
+void lru_cache_insert(lru_cache_t *pcache,
+        const char *key, const char *value, size_t value_len);
 
 #define lru_cache_tail(pcache) ((pcache)->sentinel->prev)
 

proxylab-handout/proxy.c

@@ -1,3 +1,7 @@
+/*
+ * proxy.c  -- a simple http proxy supporting GET operation
+ */
+
 #include <stdio.h>
 #include <stdlib.h>
 #include <assert.h>
@@ -6,13 +10,17 @@
 #include "bytes.h"
 #include "cache.h"
 
-#define DEBUG
+// #define DEBUG
+#undef DEBUG
 
 /* Recommended max cache and object sizes */
 #define MAX_CACHE_SIZE 1049000
 #define MAX_OBJECT_SIZE 102400
 
+/* global lru cache */
 lru_cache_t lru_cache;
+
+/* mutex to protect the lru cache update operations */
 sem_t mutex;
 
 
@@ -22,60 +30,98 @@ static const char *accept_hdr = "Accept: text/html,application/xhtml+xml,applica
 static const char *accept_encoding_hdr = "Accept-Encoding: gzip, deflate\r\n";
 
 
+/* usage */
 void usage()
 {
-    // TODO
+    printf("Usage: proxy [port]\n");
     exit(-1);
 }
 
 
 
+/* forward_response  -- forward the response back to the client
+ * key: the formatted url of the content. Used for lru_cache
+ * infd: the file descriptor of remote server. We read response from infd
+ * outfd: the client file descriptor. We write response back to outfd
+ */
 void forward_response(const char *key, int infd, int outfd)
 {
     rio_t rio;
     rio_readinitb(&rio, infd);
 
+    /* Since we don't know the length of the response, we extend
+     * the response buffer dynamically.
+     * First, we read MAXLINE number of bytes into the buffer.
+     * Then we append the buffer to the response buffer.
+     */
+
+    /* temporary buffer */
     char buf[MAXLINE];
+
+    /* response buffer */
     struct Bytes response;
+
+    /* allocate and initialize the response buffer */
     bytes_malloc(&response);
 
+    /* num_bytes is the number of bytes read for each rio_readnb operation */
     int num_bytes;
     while ((num_bytes=rio_readnb_ww(&rio, buf, MAXLINE)) > 0) {
         bytes_appendn(&response, buf, num_bytes);
     }
+
+    /* The remote connection may be closed during the read operation.
+     * The return value is < 0.
+     */
     if (num_bytes < 0) {
-        return ;
+        /* goto is used here to make sure that response is freed
+         */
+        goto FORWARD_RESPONSE_RETURN;
     }
 
 #ifdef DEBUG
     fprintf(stderr, "response length: %zu\n", bytes_length(response));
 #endif
 
+    /* the client may have closed the connection */
     if (!rio_writen_ww(outfd,
                 bytes_buf(response),
                 bytes_length(response))) {
         goto FORWARD_RESPONSE_RETURN;
     }
 
+    /* cache the response if necessary */
     if (bytes_length(response) < MAX_OBJECT_SIZE) {
         sem_wait(&mutex);
         lru_cache_insert(&lru_cache, key, bytes_buf(response),
                 bytes_length(response));
         sem_post(&mutex);
     }
 FORWARD_RESPONSE_RETURN:
+    /* free the response */
     bytes_free(&response);
 }
 
+/* forward  -- forward the request to remote server and the response from
+ * remote server
+ */
 void forward(int fromfd)
 {
     rio_t rio;
     rio_readinitb(&rio, fromfd);
-    char linebuf[MAXLINE], method[MAXLINE], uri[MAXLINE],
-         version[MAXLINE], host[MAXLINE],
-         port[MAXLINE], dir[MAXLINE],
-         request_buf[MAXBUF],
-         header_name[MAXLINE],
+    char linebuf[MAXLINE],  // temporary buffer for each line we read
+         method[MAXLINE],  // http method
+         uri[MAXLINE],  // uri
+         version[MAXLINE],  // http version.
+         host[MAXLINE],
+         port[MAXLINE],
+         dir[MAXLINE],  // the director after the host in the uri
+         /* TODO: is it necessary to use a dynamic buffer to store
+          * the request?
+          * For all the cases tested now, seems it's not necessary
+          */
+         request_buf[MAXBUF],  // The request is stored in the buffer
+         header_name[MAXLINE], // used for parsing the header
          header_value[MAXLINE];
 
     if (!rio_readlineb_ww(&rio, linebuf, MAXLINE)) {
@@ -95,7 +141,13 @@ void forward(int fromfd)
 
     char formated_uri[MAXLINE];
     sprintf(formated_uri, "%s:%s%s", host, port, dir);
+
+    /*
+     * if we find the content in the cache, we return it directly.
+     * TODO: What if the content has been modifed?
+     */
     sem_wait(&mutex);
+
     lru_cache_node_t *pnode = lru_cache_find(&lru_cache, formated_uri);
 
     if (pnode) {
@@ -109,10 +161,11 @@ void forward(int fromfd)
      * forwarded as HTTP/1.0
      */
     sprintf(request_buf, "%s %s %s\r\n", method, dir, "HTTP/1.0");
-    // sprintf(request_buf, "%s %s %s\r\n", method, dir, version);
 
+    /* if the header contains host, we forward it directly.
+     * Otherwise we fill the host according to the uri
+     */
     int has_host = 0;
-    // request headers
     if (rio_readlineb_ww(&rio, linebuf, MAXLINE) < 0) {
         return;
     }
@@ -126,18 +179,22 @@ void forward(int fromfd)
                 strcasecmp(header_name, "Accept-Encoding") == 0 ||
                 strcasecmp(header_name, "Connection") == 0 ||
                 strcasecmp(header_name, "Proxy-Connection") == 0) {
-            // ignore
+            // we have default values for these headers
         } else {
+            // for other headers, we forward it directly
             sprintf(request_buf, "%s%s", request_buf, linebuf);
         }
         if (rio_readlineb_ww(&rio, linebuf, MAXLINE) < 0) {
             return;
         }
     }
     if (!has_host) {
+        /* add host according to uri parsing result */
         sprintf(request_buf, "%s%s: %s:%s\r\n", request_buf, "Host",
                 host, port);
     }
+
+    /* add default value for these headers */
     sprintf(request_buf, "%s%s", request_buf,
             user_agent_hdr);
     sprintf(request_buf, "%s%s", request_buf,
@@ -161,13 +218,12 @@ void forward(int fromfd)
         return;
     }
 
-    // receive data
-    // read_response(clientfd);
-    
+    /* forward the response of the server to the client */
     forward_response(formated_uri, clientfd, fromfd);
     close_ww(clientfd);
 }
 
+/* thread  -- the things to do for each thread */
 void *thread(void *vargp)
 {
     int connfd = *((int*)vargp);
@@ -179,12 +235,18 @@ void *thread(void *vargp)
 }
 
 
+/* sigpipe_handler  -- handle sig pipe
+ * SIGPIPE is generated when the connection is closed by another end early.
+ * */
 void sigpipe_handler(int sig)
 {
+#ifdef DEBUG
     fprintf(stderr, "[WARNING] Catch a sigpipe signal\n");
+#endif
 }
 
 
+/* main */
 int main(int argc, char **argv)
 {
     if (argc != 2) {