about summary refs log tree commit homepage
diff options
context:
space:
mode:
authorEric Wong <e@80x24.org>2017-04-17 23:22:56 +0000
committerEric Wong <e@80x24.org>2018-12-01 13:26:44 +0000
commit63dffb599fc5d0e6270d1bf68191de86c255b37f (patch)
tree0632dcca9195fb0a2247edf4959ed95f59445275
parent15fe042f13ab9648858f4e96f227b7b5b9ad9694 (diff)
downloadkcar-63dffb599fc5d0e6270d1bf68191de86c255b37f.tar.gz
This (IMHO) improves readability of code in the face of future
changes.  At least on my system(*), binary size is even reduced:

	 text      data     bss     dec     hex filename
	59233      1088     136   60457    ec29 before.so
	58945      1088     136   60169    eb09 after.so

(*) x86-64 Debian gcc 4.9.2-10
-rw-r--r--ext/kcar/kcar.rl87
-rw-r--r--test/test_parser.rb1
2 files changed, 41 insertions, 47 deletions
diff --git a/ext/kcar/kcar.rl b/ext/kcar/kcar.rl
index 033e6ea..c0e8bbc 100644
--- a/ext/kcar/kcar.rl
+++ b/ext/kcar/kcar.rl
@@ -49,19 +49,17 @@ DEF_MAX_LENGTH(FRAGMENT, 1024); /* just in case (stolen from Mongrel) */
 DEF_MAX_LENGTH(REQUEST_PATH, 4096); /* common PATH_MAX on modern systems */
 DEF_MAX_LENGTH(QUERY_STRING, (1024 * 10));
 
-
-#define UH_FL_CHUNKED  0x1
-#define UH_FL_HASBODY  0x2
-#define UH_FL_INBODY   0x4
-#define UH_FL_HASTRAILER   0x8
-#define UH_FL_INTRAILER 0x10
-#define UH_FL_INCHUNK  0x20
-#define UH_FL_KEEPALIVE 0x40
-#define UH_FL_HASHEADER 0x80
-
 struct http_parser {
   int cs; /* Ragel internal state */
-  unsigned int flags;
+  unsigned int chunked:1;
+  unsigned int has_body:1;
+  unsigned int in_body:1;
+  unsigned int has_trailer:1;
+  unsigned int in_trailer:1;
+  unsigned int in_chunk:1;
+  unsigned int persistent:1;
+  unsigned int has_header:1;
+  unsigned int padding:24;
   unsigned int mark;
   unsigned int offset;
   union { /* these 2 fields don't nest */
@@ -99,11 +97,6 @@ static unsigned int ulong2uint(unsigned long n)
 #define STR_NEW(M,FPC) rb_str_new(PTR_TO(M), LEN(M, FPC))
 #define STRIPPED_STR_NEW(M,FPC) stripped_str_new(PTR_TO(M), LEN(M, FPC))
 
-#define HP_FL_TEST(hp,fl) ((hp)->flags & (UH_FL_##fl))
-#define HP_FL_SET(hp,fl) ((hp)->flags |= (UH_FL_##fl))
-#define HP_FL_UNSET(hp,fl) ((hp)->flags &= ~(UH_FL_##fl))
-#define HP_FL_ALL(hp,fl) (HP_FL_TEST(hp, fl) == (UH_FL_##fl))
-
 /* Downcases a single ASCII character.  Locale-agnostic. */
 static void downcase_char(char *c)
 {
@@ -143,7 +136,7 @@ static VALUE stripped_str_new(const char *str, long len)
 
 static void finalize_header(struct http_parser *hp)
 {
-  if ((HP_FL_TEST(hp, HASTRAILER) && ! HP_FL_TEST(hp, CHUNKED)))
+  if (hp->has_trailer && !hp->chunked)
     rb_raise(eParserError, "trailer but not chunked");
 }
 
@@ -157,17 +150,17 @@ static void hp_keepalive_connection(struct http_parser *hp, VALUE val)
   /* REQUEST_METHOD is always set before any headers */
   if (STR_CSTR_CASE_EQ(val, "keep-alive")) {
     /* basically have HTTP/1.0 masquerade as HTTP/1.1+ */
-    HP_FL_SET(hp, KEEPALIVE);
+    hp->persistent = 1;
   } else if (STR_CSTR_CASE_EQ(val, "close")) {
     /*
      * it doesn't matter what HTTP version or request method we have,
      * if a server says "Connection: close", we disable keepalive
      */
-    HP_FL_UNSET(hp, KEEPALIVE);
+    hp->persistent = 0;
   } else {
     /*
      * server could've sent anything, ignore it for now.  Maybe
-     * "HP_FL_UNSET(hp, KEEPALIVE);" just in case?
+     * "hp->persistent = 0;" just in case?
      * Raising an exception might be too mean...
      */
   }
@@ -225,7 +218,7 @@ http_version(struct http_parser *hp, const char *ptr, size_t len)
 {
   if (CONST_MEM_EQ("HTTP/1.1", ptr, len)) {
     /* HTTP/1.1 implies keepalive unless "Connection: close" is set */
-    HP_FL_SET(hp, KEEPALIVE);
+    hp->persistent = 1;
   }
 }
 
@@ -243,12 +236,12 @@ status_phrase(struct http_parser *hp, VALUE hdr, const char *ptr, size_t len)
     rb_raise(eParserError, "invalid status: %s", RSTRING_PTR(hp->status));
 
   if ( !((nr >= 100 && nr <= 199) || nr == 204 || nr == 304) )
-    HP_FL_SET(hp, HASBODY);
+    hp->has_body = 1;
 }
 
 static inline void invalid_if_trailer(struct http_parser *hp)
 {
-  if (HP_FL_TEST(hp, INTRAILER))
+  if (hp->in_trailer)
     rb_raise(eParserError, "invalid Trailer");
 }
 
@@ -299,7 +292,7 @@ static void write_value(VALUE hdr, struct http_parser *hp,
   const char *vptr;
   size_t vlen;
 
-  HP_FL_SET(hp, HASHEADER);
+  hp->has_header = 1;
 
   /* Rack does not like Status headers, so we never send them */
   if (CSTR_CASE_EQ(fptr, flen, "status")) {
@@ -323,9 +316,9 @@ static void write_value(VALUE hdr, struct http_parser *hp,
   if (STR_CSTR_CASE_EQ(f, "connection")) {
     hp_keepalive_connection(hp, v);
   } else if (STR_CSTR_CASE_EQ(f, "content-length")) {
-    if (! HP_FL_TEST(hp, HASBODY))
+    if (!hp->has_body)
       rb_raise(eParserError, "Content-Length with no body expected");
-    if (HP_FL_TEST(hp, CHUNKED))
+    if (hp->chunked)
       rb_raise(eParserError,
                "Content-Length when chunked Transfer-Encoding is set");
     hp->len.content = parse_length(vptr, vlen);
@@ -336,7 +329,7 @@ static void write_value(VALUE hdr, struct http_parser *hp,
     invalid_if_trailer(hp);
   } else if (STR_CSTR_CASE_EQ(f, "transfer-encoding")) {
     if (STR_CSTR_CASE_EQ(v, "chunked")) {
-      if (! HP_FL_TEST(hp, HASBODY))
+      if (!hp->has_body)
         rb_raise(eParserError,
                  "chunked Transfer-Encoding with no body expected");
       if (hp->len.content >= 0)
@@ -344,13 +337,13 @@ static void write_value(VALUE hdr, struct http_parser *hp,
                  "chunked Transfer-Encoding when Content-Length is set");
 
       hp->len.chunk = 0;
-      HP_FL_SET(hp, CHUNKED);
+      hp->chunked = 1;
     }
     invalid_if_trailer(hp);
   } else if (STR_CSTR_CASE_EQ(f, "trailer")) {
-    if (! HP_FL_TEST(hp, HASBODY))
+    if (!hp->has_body)
       rb_raise(eParserError, "trailer with no body");
-    HP_FL_SET(hp, HASTRAILER);
+    hp->has_trailer = 1;
     invalid_if_trailer(hp);
   }
 
@@ -425,7 +418,7 @@ static void write_value(VALUE hdr, struct http_parser *hp,
     finalize_header(hp);
     cs = http_parser_first_final;
 
-    if (HP_FL_TEST(hp, CHUNKED))
+    if (hp->chunked)
       cs = http_parser_en_ChunkedBody;
 
     /*
@@ -441,7 +434,7 @@ static void write_value(VALUE hdr, struct http_parser *hp,
   }
 
   action end_chunked_body {
-    HP_FL_SET(hp, INTRAILER);
+    hp->in_trailer = 1;
     cs = http_parser_en_Trailers;
     ++p;
     assert(p <= pe && "buffer overflow after chunked body");
@@ -457,7 +450,7 @@ static void write_value(VALUE hdr, struct http_parser *hp,
     p += nr;
     assert(hp->len.chunk >= 0 && "negative chunk length");
     if ((size_t)hp->len.chunk > REMAINING) {
-      HP_FL_SET(hp, INCHUNK);
+      hp->in_chunk = 1;
       goto post_exec;
     } else {
       fhold;
@@ -501,8 +494,8 @@ static void http_parser_execute(struct http_parser *hp,
   assert((void *)(pe - p) == (void *)(len - off) &&
          "pointers aren't same distance");
 
-  if (HP_FL_TEST(hp, INCHUNK)) {
-    HP_FL_UNSET(hp, INCHUNK);
+  if (hp->in_chunk) {
+    hp->in_chunk = 0;
     goto skip_chunk_data_hack;
   }
   %% write exec;
@@ -603,7 +596,7 @@ static VALUE body_bytes_left(VALUE self)
 {
   struct http_parser *hp = data_get(self);
 
-  if (HP_FL_TEST(hp, CHUNKED))
+  if (hp->chunked)
     return Qnil;
   if (hp->len.content >= 0)
     return OFFT2NUM(hp->len.content);
@@ -623,7 +616,7 @@ static VALUE body_bytes_left_set(VALUE self, VALUE bytes)
 {
   struct http_parser *hp = data_get(self);
 
-  if (HP_FL_TEST(hp, CHUNKED))
+  if (hp->chunked)
     rb_raise(rb_eRuntimeError, "body_bytes_left= is not for chunked bodies");
   hp->len.content = NUM2OFFT(bytes);
   return bytes;
@@ -640,7 +633,7 @@ static VALUE chunked(VALUE self)
 {
   struct http_parser *hp = data_get(self);
 
-  return HP_FL_TEST(hp, CHUNKED) ? Qtrue : Qfalse;
+  return hp->chunked ? Qtrue : Qfalse;
 }
 
 static void check_buffer_size(long dlen)
@@ -674,7 +667,7 @@ static VALUE headers(VALUE self, VALUE hdr, VALUE data)
       hp->cs == http_parser_en_ChunkedBody) {
     advance_str(data, hp->offset + 1);
     hp->offset = 0;
-    if (HP_FL_TEST(hp, INTRAILER))
+    if (hp->in_trailer)
       return hdr;
     else
       return rb_ary_new3(2, hp->status, hdr);
@@ -688,7 +681,7 @@ static VALUE headers(VALUE self, VALUE hdr, VALUE data)
 
 static int chunked_eof(struct http_parser *hp)
 {
-  return ((hp->cs == http_parser_first_final) || HP_FL_TEST(hp, INTRAILER));
+  return ((hp->cs == http_parser_first_final) || hp->in_trailer);
 }
 
 /**
@@ -702,13 +695,13 @@ static VALUE body_eof(VALUE self)
 {
   struct http_parser *hp = data_get(self);
 
-  if (!HP_FL_TEST(hp, HASHEADER) && HP_FL_ALL(hp, KEEPALIVE))
+  if (!hp->has_header && hp->persistent)
     return Qtrue;
 
-  if (HP_FL_TEST(hp, CHUNKED))
+  if (hp->chunked)
     return chunked_eof(hp) ? Qtrue : Qfalse;
 
-  if (! HP_FL_TEST(hp, HASBODY))
+  if (!hp->has_body)
     return Qtrue;
 
   return hp->len.content == 0 ? Qtrue : Qfalse;
@@ -730,9 +723,9 @@ static VALUE keepalive(VALUE self)
 {
   struct http_parser *hp = data_get(self);
 
-  if (HP_FL_ALL(hp, KEEPALIVE)) {
-    if (HP_FL_TEST(hp, HASHEADER) && HP_FL_TEST(hp, HASBODY) ) {
-      if (HP_FL_TEST(hp, CHUNKED) || (hp->len.content >= 0))
+  if (hp->persistent) {
+    if (hp->has_header && hp->has_body) {
+      if (hp->chunked || (hp->len.content >= 0))
         return Qtrue;
 
       /* unknown Content-Length and not chunked, we must assume close */
@@ -774,7 +767,7 @@ static VALUE filter_body(VALUE self, VALUE buf, VALUE data)
   rb_str_resize(buf, dlen); /* we can never copy more than dlen bytes */
   OBJ_TAINT(buf); /* keep weirdo $SAFE users happy */
 
-  if (!HP_FL_TEST(hp, CHUNKED))
+  if (!hp->chunked)
     rb_raise(rb_eRuntimeError, "filter_body is only for chunked bodies");
 
   if (!chunked_eof(hp)) {
diff --git a/test/test_parser.rb b/test/test_parser.rb
index 3768808..84e609d 100644
--- a/test/test_parser.rb
+++ b/test/test_parser.rb
@@ -300,6 +300,7 @@ class TestParser < Test::Unit::TestCase
     require 'objspace'
     n = ObjectSpace.memsize_of(@hp)
     assert_kind_of Integer, n
+    warn "memsize: #{n}\n" if $DEBUG
   rescue LoadError
     warn 'ObjectSpace not available'
   end