cookies: refactor comments

Comments in the cookie code were a bit all over the place in terms of
style and wording. This takes a stab at cleaning them up by keeping to
a single style and overall shape. Some comments are moved a little and
some removed alltogether due to being redundant. No functional changes
have been made,

1 files changed, 207 insertions, 151 deletions

diff --git a/lib/cookie.c b/lib/cookie.c
index 8831b2a0e..082a32f4c 100644
--- a/lib/cookie.c
+++ b/lib/cookie.c
@@ -129,12 +129,13 @@ static bool tailmatch(const char *cooke_domain, const char *hostname)
   if(!strcasecompare(cooke_domain, hostname + hostname_len-cookie_domain_len))
     return FALSE;
 
-  /* A lead char of cookie_domain is not '.'.
-     RFC6265 4.1.2.3. The Domain Attribute says:
-       For example, if the value of the Domain attribute is
-       "example.com", the user agent will include the cookie in the Cookie
-       header when making HTTP requests to example.com, www.example.com, and
-       www.corp.example.com.
+  /*
+   * A lead char of cookie_domain is not '.'.
+   * RFC6265 4.1.2.3. The Domain Attribute says:
+   * For example, if the value of the Domain attribute is
+   * "example.com", the user agent will include the cookie in the Cookie
+   * header when making HTTP requests to example.com, www.example.com, and
+   * www.corp.example.com.
    */
   if(hostname_len == cookie_domain_len)
     return TRUE;
@@ -144,7 +145,10 @@ static bool tailmatch(const char *cooke_domain, const char *hostname)
 }
 
 /*
- * Return true if the given string is an IP(v4|v6) address.
+ * isip
+ *
+ * Returns true if the given string is an IPv4 or IPv6 address (if IPv6 has
+ * been enabled while building libcurl, and false otherwise.
  */
 static bool isip(const char *domain)
 {
@@ -199,13 +203,14 @@ static bool pathmatch(const char *cookie_path, const char *request_uri)
       return FALSE;
   }
 
-  /* here, RFC6265 5.1.4 says
-     4. Output the characters of the uri-path from the first character up
-        to, but not including, the right-most %x2F ("/").
-     but URL path /hoge?fuga=xxx means /hoge/index.cgi?fuga=xxx in some site
-     without redirect.
-     Ignore this algorithm because /hoge is uri path for this case
-     (uri path is not /).
+  /*
+   * here, RFC6265 5.1.4 says
+   *  4. Output the characters of the uri-path from the first character up
+   *     to, but not including, the right-most %x2F ("/").
+   *  but URL path /hoge?fuga=xxx means /hoge/index.cgi?fuga=xxx in some site
+   *  without redirect.
+   *  Ignore this algorithm because /hoge is uri path for this case
+   *  (uri path is not /).
    */
 
   uri_path_len = strlen(uri_path);
@@ -357,7 +362,8 @@ void Curl_cookie_loadfiles(struct Curl_easy *data)
                                         data->cookies,
                                         data->set.cookiesession);
       if(!newcookies)
-        /* Failure may be due to OOM or a bad cookie; both are ignored
+        /*
+         * Failure may be due to OOM or a bad cookie; both are ignored
          * but only the first should be
          */
         infof(data, "ignoring failed cookie_init for %s\n", list->data);
@@ -372,10 +378,13 @@ void Curl_cookie_loadfiles(struct Curl_easy *data)
 }
 
 /*
- * strstore() makes a strdup() on the 'newstr' and if '*str' is non-NULL
- * that will be freed before the allocated string is stored there.
+ * strstore
  *
- * It is meant to easily replace strdup()
+ * A thin wrapper around strdup which ensures that any memory allocated at
+ * *str will be freed before the string allocated by strdup is stored there.
+ * The intended usecase is repeated assignments to the same variable during
+ * parsing in a last-wins scenario. The caller is responsible for checking
+ * for OOM errors.
  */
 static void strstore(char **str, const char *newstr)
 {
@@ -384,7 +393,11 @@ static void strstore(char **str, const char *newstr)
 }
 
 /*
- * remove_expired() removes expired cookies.
+ * remove_expired
+ *
+ * Remove expired cookies from the hash by inspecting the expires timestamp on
+ * each cookie in the hash, freeing and deleting any where the timestamp is in
+ * the past.
  */
 static void remove_expired(struct CookieInfo *cookies)
 {
@@ -421,25 +434,23 @@ static bool bad_domain(const char *domain)
   return !strchr(domain, '.') && !strcasecompare(domain, "localhost");
 }
 
-/****************************************************************************
- *
- * Curl_cookie_add()
- *
- * Add a single cookie line to the cookie keeping object.
+/*
+ * Curl_cookie_add
  *
- * Be aware that sometimes we get an IP-only host name, and that might also be
- * a numerical IPv6 address.
+ * Add a single cookie line to the cookie keeping object. Be aware that
+ * sometimes we get an IP-only host name, and that might also be a numerical
+ * IPv6 address.
  *
  * Returns NULL on out of memory or invalid cookie. This is suboptimal,
  * as they should be treated separately.
- ***************************************************************************/
-
+ */
 struct Cookie *
 Curl_cookie_add(struct Curl_easy *data,
-                /* The 'data' pointer here may be NULL at times, and thus
-                   must only be used very carefully for things that can deal
-                   with data being NULL. Such as infof() and similar */
-
+                /*
+                 * The 'data' pointer here may be NULL at times, and thus
+                 * must only be used very carefully for things that can deal
+                 * with data being NULL. Such as infof() and similar
+                 */
                 struct CookieInfo *c,
                 bool httpheader, /* TRUE if HTTP header-style line */
                 bool noexpire, /* if TRUE, skip remove_expired() */
@@ -493,9 +504,11 @@ Curl_cookie_add(struct Curl_easy *data,
       if(1 <= sscanf(ptr, "%" MAX_NAME_TXT "[^;\r\n=] =%"
                      MAX_NAME_TXT "[^;\r\n]",
                      name, what)) {
-        /* Use strstore() below to properly deal with received cookie
-           headers that have the same string property set more than once,
-           and then we use the last one. */
+        /*
+         * Use strstore() below to properly deal with received cookie
+         * headers that have the same string property set more than once,
+         * and then we use the last one.
+         */
         const char *whatptr;
         bool done = FALSE;
         bool sep;
@@ -503,11 +516,13 @@ Curl_cookie_add(struct Curl_easy *data,
         size_t nlen = strlen(name);
         const char *endofn = &ptr[ nlen ];
 
+        /*
+         * Check for too long individual name or contents, or too long
+         * combination of name + contents. Chrome and Firefox support 4095 or
+         * 4096 bytes combo
+         */
         if(nlen >= (MAX_NAME-1) || len >= (MAX_NAME-1) ||
            ((nlen + len) > MAX_NAME)) {
-          /* too long individual name or contents, or too long combination of
-             name + contents. Chrome and Firefox support 4095 or 4096 bytes
-             combo. */
           freecookie(co);
           infof(data, "oversized cookie dropped, name/val %zu + %zu bytes\n",
                 nlen, len);
@@ -569,8 +584,10 @@ Curl_cookie_add(struct Curl_easy *data,
           }
         }
         else if(!len) {
-          /* this was a "<name>=" with no content, and we must allow
-             'secure' and 'httponly' specified this weirdly */
+          /*
+           * this was a "<name>=" with no content, and we must allow
+           * 'secure' and 'httponly' specified this weirdly
+           */
           done = TRUE;
           /*
            * secure cookies are only allowed to be set when the connection is
@@ -610,8 +627,10 @@ Curl_cookie_add(struct Curl_easy *data,
         else if(strcasecompare("domain", name)) {
           bool is_ip;
 
-          /* Now, we make sure that our host is within the given domain,
-             or the given domain is not valid and thus cannot be set. */
+          /*
+           * Now, we make sure that our host is within the given domain, or
+           * the given domain is not valid and thus cannot be set.
+           */
 
           if('.' == whatptr[0])
             whatptr++; /* ignore preceding dot */
@@ -641,9 +660,10 @@ Curl_cookie_add(struct Curl_easy *data,
                                        given */
           }
           else {
-            /* we did not get a tailmatch and then the attempted set domain
-               is not a domain to which the current host belongs. Mark as
-               bad. */
+            /*
+             * We did not get a tailmatch and then the attempted set domain is
+             * not a domain to which the current host belongs. Mark as bad.
+             */
             badcookie = TRUE;
             infof(data, "skipped cookie with bad tailmatch domain: %s\n",
                   whatptr);
@@ -657,15 +677,15 @@ Curl_cookie_add(struct Curl_easy *data,
           }
         }
         else if(strcasecompare("max-age", name)) {
-          /* Defined in RFC2109:
-
-             Optional.  The Max-Age attribute defines the lifetime of the
-             cookie, in seconds.  The delta-seconds value is a decimal non-
-             negative integer.  After delta-seconds seconds elapse, the
-             client should discard the cookie.  A value of zero means the
-             cookie should be discarded immediately.
-
-          */
+          /*
+           * Defined in RFC2109:
+           *
+           * Optional.  The Max-Age attribute defines the lifetime of the
+           * cookie, in seconds.  The delta-seconds value is a decimal non-
+           * negative integer.  After delta-seconds seconds elapse, the
+           * client should discard the cookie.  A value of zero means the
+           * cookie should be discarded immediately.
+           */
           strstore(&co->maxage, whatptr);
           if(!co->maxage) {
             badcookie = TRUE;
@@ -679,9 +699,10 @@ Curl_cookie_add(struct Curl_easy *data,
             break;
           }
         }
+
         /*
-          else this is the second (or more) name we don't know
-          about! */
+         * Else, this is the second (or more) name we don't know about!
+         */
       }
       else {
         /* this is an "illegal" <what>=<this> pair */
@@ -699,8 +720,10 @@ Curl_cookie_add(struct Curl_easy *data,
       semiptr = strchr(ptr, ';'); /* now, find the next semicolon */
 
       if(!semiptr && *ptr)
-        /* There are no more semicolons, but there's a final name=value pair
-           coming up */
+        /*
+         * There are no more semicolons, but there's a final name=value pair
+         * coming up
+         */
         semiptr = strchr(ptr, '\0');
     } while(semiptr);
 
@@ -724,13 +747,16 @@ Curl_cookie_add(struct Curl_easy *data,
       }
     }
     else if(co->expirestr) {
-      /* Note that if the date couldn't get parsed for whatever reason,
-         the cookie will be treated as a session cookie */
+      /*
+       * Note that if the date couldn't get parsed for whatever reason, the
+       * cookie will be treated as a session cookie
+       */
       co->expires = Curl_getdate_capped(co->expirestr);
 
-      /* Session cookies have expires set to 0 so if we get that back
-         from the date parser let's add a second to make it a
-         non-session cookie */
+      /*
+       * Session cookies have expires set to 0 so if we get that back from the
+       * date parser let's add a second to make it a non-session cookie
+       */
       if(co->expires == 0)
         co->expires = 1;
       else if(co->expires < 0)
@@ -747,13 +773,17 @@ Curl_cookie_add(struct Curl_easy *data,
     }
 
     if(!badcookie && !co->path && path) {
-      /* No path was given in the header line, set the default.
-         Note that the passed-in path to this function MAY have a '?' and
-         following part that MUST not be stored as part of the path. */
+      /*
+       * No path was given in the header line, set the default.  Note that the
+       * passed-in path to this function MAY have a '?' and following part that
+       * MUST NOT be stored as part of the path.
+       */
       char *queryp = strchr(path, '?');
 
-      /* queryp is where the interesting part of the path ends, so now we
-         want to the find the last */
+      /*
+       * queryp is where the interesting part of the path ends, so now we
+       * want to the find the last
+       */
       char *endslash;
       if(!queryp)
         endslash = strrchr(path, '/');
@@ -774,29 +804,34 @@ Curl_cookie_add(struct Curl_easy *data,
       }
     }
 
+    /*
+     * If we didn't get a cookie name, or a bad one, the this is an illegal
+     * line so bail out.
+     */
     if(badcookie || !co->name) {
-      /* we didn't get a cookie name or a bad one,
-         this is an illegal line, bail out */
       freecookie(co);
       return NULL;
     }
 
   }
   else {
-    /* This line is NOT a HTTP header style line, we do offer support for
-       reading the odd netscape cookies-file format here */
+    /*
+     * This line is NOT a HTTP header style line, we do offer support for
+     * reading the odd netscape cookies-file format here
+     */
     char *ptr;
     char *firstptr;
     char *tok_buf = NULL;
     int fields;
 
-    /* IE introduced HTTP-only cookies to prevent XSS attacks. Cookies
-       marked with httpOnly after the domain name are not accessible
-       from javascripts, but since curl does not operate at javascript
-       level, we include them anyway. In Firefox's cookie files, these
-       lines are preceded with #HttpOnly_ and then everything is
-       as usual, so we skip 10 characters of the line..
-    */
+    /*
+     * IE introduced HTTP-only cookies to prevent XSS attacks. Cookies marked
+     * with httpOnly after the domain name are not accessible from javascripts,
+     * but since curl does not operate at javascript level, we include them
+     * anyway. In Firefox's cookie files, these lines are preceded with
+     * #HttpOnly_ and then everything is as usual, so we skip 10 characters of
+     * the line..
+     */
     if(strncmp(lineptr, "#HttpOnly_", 10) == 0) {
       lineptr += 10;
       co->httponly = TRUE;
@@ -817,8 +852,10 @@ Curl_cookie_add(struct Curl_easy *data,
 
     firstptr = strtok_r(lineptr, "\t", &tok_buf); /* tokenize it on the TAB */
 
-    /* Now loop through the fields and init the struct we already have
-       allocated */
+    /*
+     * Now loop through the fields and init the struct we already have
+     * allocated
+     */
     for(ptr = firstptr, fields = 0; ptr && !badcookie;
         ptr = strtok_r(NULL, "\t", &tok_buf), fields++) {
       switch(fields) {
@@ -830,10 +867,11 @@ Curl_cookie_add(struct Curl_easy *data,
           badcookie = TRUE;
         break;
       case 1:
-        /* flag: A TRUE/FALSE value indicating if all machines within a given
-           domain can access the variable. Set TRUE when the cookie says
-           .domain.com and to false when the domain is complete www.domain.com
-        */
+        /*
+         * flag: A TRUE/FALSE value indicating if all machines within a given
+         * domain can access the variable. Set TRUE when the cookie says
+         * .domain.com and to false when the domain is complete www.domain.com
+         */
         co->tailmatch = strcasecompare(ptr, "TRUE")?TRUE:FALSE;
         break;
       case 2:
@@ -942,9 +980,11 @@ Curl_cookie_add(struct Curl_easy *data,
   co->livecookie = c->running;
   co->creationtime = ++c->lastct;
 
-  /* now, we have parsed the incoming line, we must now check if this
-     supersedes an already existing cookie, which it may if the previous have
-     the same domain and path as this */
+  /*
+   * Now we have parsed the incoming line, we must now check if this supersedes
+   * an already existing cookie, which it may if the previous have the same
+   * domain and path as this.
+   */
 
   /* at first, remove expired cookies */
   if(!noexpire)
@@ -1032,12 +1072,12 @@ Curl_cookie_add(struct Curl_easy *data,
       }
 
       if(replace_old && !co->livecookie && clist->livecookie) {
-        /* Both cookies matched fine, except that the already present
-           cookie is "live", which means it was set from a header, while
-           the new one isn't "live" and thus only read from a file. We let
-           live cookies stay alive */
-
-        /* Free the newcomer and get out of here! */
+        /*
+         * Both cookies matched fine, except that the already present cookie is
+         * "live", which means it was set from a header, while the new one was
+         * read from a file and thus isn't "live". "live" cookies are preferred
+         * so the new cookie is freed.
+         */
         freecookie(co);
         return NULL;
       }
@@ -1063,8 +1103,10 @@ Curl_cookie_add(struct Curl_easy *data,
         free(co);   /* free the newly allocated memory */
         co = clist; /* point to the previous struct instead */
 
-        /* We have replaced a cookie, now skip the rest of the list but
-           make sure the 'lastc' pointer is properly set */
+        /*
+         * We have replaced a cookie, now skip the rest of the list but make
+         * sure the 'lastc' pointer is properly set
+         */
         do {
           lastc = clist;
           clist = clist->next;
@@ -1096,19 +1138,19 @@ Curl_cookie_add(struct Curl_easy *data,
 }
 
 
-/*****************************************************************************
- *
+/*
  * Curl_cookie_init()
  *
  * Inits a cookie struct to read data from a local file. This is always
- * called before any cookies are set. File may be NULL.
+ * called before any cookies are set. File may be NULL in which case only the
+ * struct is initialized. Is file is "-" then STDIN is read.
  *
  * If 'newsession' is TRUE, discard all "session cookies" on read from file.
  *
  * Note that 'data' might be called as NULL pointer.
  *
  * Returns NULL on out of memory. Invalid cookies are ignored.
- ****************************************************************************/
+ */
 struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
                                     const char *file,
                                     struct CookieInfo *inc,
@@ -1170,7 +1212,12 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
       Curl_cookie_add(data, c, headerline, TRUE, lineptr, NULL, NULL, TRUE);
     }
     free(line); /* free the line buffer */
-    remove_expired(c); /* run this once, not on every cookie */
+
+    /*
+     * Remove expired cookies from the hash. We must make sure to run this
+     * after reading the file, and not not on every cookie.
+     */
+    remove_expired(c);
 
     if(fromfile)
       fclose(fp);
@@ -1184,16 +1231,24 @@ struct CookieInfo *Curl_cookie_init(struct Curl_easy *data,
 
 fail:
   free(line);
+  /*
+   * Only clean up if we allocated it here, as the original could still be in
+   * use by a share handle.
+   */
   if(!inc)
-    /* Only clean up if we allocated it here, as the original could still be in
-     * use by a share handle */
     Curl_cookie_cleanup(c);
   if(fromfile && fp)
     fclose(fp);
   return NULL; /* out of memory */
 }
 
-/* sort this so that the longest path gets before the shorter path */
+/*
+ * cookie_sort
+ *
+ * Helper function to sort cookies such that the longest path gets before the
+ * shorter path. Path, domain and name lengths are considered in that order,
+ * with tge creationtime as the tiebreaker.
+ */
 static int cookie_sort(const void *p1, const void *p2)
 {
   struct Cookie *c1 = *(struct Cookie **)p1;
@@ -1225,7 +1280,11 @@ static int cookie_sort(const void *p1, const void *p2)
   return (c2->creationtime > c1->creationtime) ? 1 : -1;
 }
 
-/* sort cookies only according to creation time */
+/*
+ * cookie_sort_ct
+ *
+ * Helper function to sort cookies according to creation time.
+ */
 static int cookie_sort_ct(const void *p1, const void *p2)
 {
   struct Cookie *c1 = *(struct Cookie **)p1;
@@ -1269,18 +1328,15 @@ static struct Cookie *dup_cookie(struct Cookie *src)
   return NULL;
 }
 
-/*****************************************************************************
- *
- * Curl_cookie_getlist()
+/*
+ * Curl_cookie_getlist
  *
- * For a given host and path, return a linked list of cookies that the
- * client should send to the server if used now. The secure boolean informs
- * the cookie if a secure connection is achieved or not.
+ * For a given host and path, return a linked list of cookies that the client
+ * should send to the server if used now. The secure boolean informs the cookie
+ * if a secure connection is achieved or not.
  *
  * It shall only return cookies that haven't expired.
- *
- ****************************************************************************/
-
+ */
 struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
                                    const char *host, const char *path,
                                    bool secure)
@@ -1311,15 +1367,21 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
       if(!co->domain ||
          (co->tailmatch && !is_ip && tailmatch(co->domain, host)) ||
          ((!co->tailmatch || is_ip) && strcasecompare(host, co->domain)) ) {
-        /* the right part of the host matches the domain stuff in the
-           cookie data */
+        /*
+         * the right part of the host matches the domain stuff in the
+         * cookie data
+         */
 
-        /* now check the left part of the path with the cookies path
-           requirement */
+        /*
+         * now check the left part of the path with the cookies path
+         * requirement
+         */
         if(!co->spath || pathmatch(co->spath, path) ) {
 
-          /* and now, we know this is a match and we should create an
-             entry for the return-linked-list */
+          /*
+           * and now, we know this is a match and we should create an
+           * entry for the return-linked-list
+           */
 
           newco = dup_cookie(co);
           if(newco) {
@@ -1340,9 +1402,11 @@ struct Cookie *Curl_cookie_getlist(struct CookieInfo *c,
   }
 
   if(matches) {
-    /* Now we need to make sure that if there is a name appearing more than
-       once, the longest specified path version comes first. To make this
-       the swiftest way, we just sort them all based on path length. */
+    /*
+     * Now we need to make sure that if there is a name appearing more than
+     * once, the longest specified path version comes first. To make this
+     * the swiftest way, we just sort them all based on path length.
+     */
     struct Cookie **array;
     size_t i;
 
@@ -1377,13 +1441,11 @@ fail:
   return NULL;
 }
 
-/*****************************************************************************
- *
- * Curl_cookie_clearall()
+/*
+ * Curl_cookie_clearall
  *
  * Clear all existing cookies and reset the counter.
- *
- ****************************************************************************/
+ */
 void Curl_cookie_clearall(struct CookieInfo *cookies)
 {
   if(cookies) {
@@ -1396,14 +1458,11 @@ void Curl_cookie_clearall(struct CookieInfo *cookies)
   }
 }
 
-/*****************************************************************************
- *
- * Curl_cookie_freelist()
+/*
+ * Curl_cookie_freelist
  *
  * Free a list of cookies previously returned by Curl_cookie_getlist();
- *
- ****************************************************************************/
-
+ */
 void Curl_cookie_freelist(struct Cookie *co)
 {
   struct Cookie *next;
@@ -1414,14 +1473,11 @@ void Curl_cookie_freelist(struct Cookie *co)
   }
 }
 
-
-/*****************************************************************************
- *
- * Curl_cookie_clearsess()
+/*
+ * Curl_cookie_clearsess
  *
  * Free all session cookies in the cookies list.
- *
- ****************************************************************************/
+ */
 void Curl_cookie_clearsess(struct CookieInfo *cookies)
 {
   struct Cookie *first, *curr, *next, *prev = NULL;
@@ -1458,14 +1514,11 @@ void Curl_cookie_clearsess(struct CookieInfo *cookies)
   }
 }
 
-
-/*****************************************************************************
- *
+/*
  * Curl_cookie_cleanup()
  *
  * Free a "cookie object" previous created with Curl_cookie_init().
- *
- ****************************************************************************/
+ */
 void Curl_cookie_cleanup(struct CookieInfo *c)
 {
   if(c) {
@@ -1477,12 +1530,13 @@ void Curl_cookie_cleanup(struct CookieInfo *c)
   }
 }
 
-/* get_netscape_format()
+/*
+ * get_netscape_format()
  *
  * Formats a string for Netscape output file, w/o a newline at the end.
- *
- * Function returns a char * to a formatted line. Has to be free()d
-*/
+ * Function returns a char * to a formatted line. The caller is responsible
+ * for freeing the returned pointer.
+ */
 static char *get_netscape_format(const struct Cookie *co)
 {
   return aprintf(
@@ -1495,8 +1549,10 @@ static char *get_netscape_format(const struct Cookie *co)
     "%s\t"   /* name */
     "%s",    /* value */
     co->httponly?"#HttpOnly_":"",
-    /* Make sure all domains are prefixed with a dot if they allow
-       tailmatching. This is Mozilla-style. */
+    /*
+     * Make sure all domains are prefixed with a dot if they allow
+     * tailmatching. This is Mozilla-style.
+     */
     (co->tailmatch && co->domain && co->domain[0] != '.')? ".":"",
     co->domain?co->domain:"unknown",
     co->tailmatch?"TRUE":"FALSE",