Minor improvements to http linux client. Added const in places, a few std::move calls, fixed some indentation.

casaserv · casaserv · commit 1cad17f73206 · 2014-04-06T22:14:07.000-07:00
In general this code could be significantly improved if we fixed all the duplicated socket/ssl code paths.
diff --git a/Release/src/http/client/http_linux.cpp b/Release/src/http/client/http_linux.cpp
@@ -52,8 +52,6 @@ namespace web { namespace http
                 close
             };
 
-            class linux_client;
-
             class linux_request_context : public request_context
             {
             public:
@@ -127,6 +125,7 @@ namespace web { namespace http
                 }
 
                 linux_request_context(std::shared_ptr<_http_client_communicator> &client, http_request request);
+
             protected:
                 virtual void cleanup()
                 {
@@ -365,7 +364,7 @@ namespace web { namespace http
                             ctx->report_exception(std::current_exception());
                             return;
                         }
-                        size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space, readSize);
+                        const size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space, readSize);
                         ctx->m_body_buf.commit(readSize + http::details::chunked_encoding::additional_encoding_space);
                         ctx->m_body_buf.consume(offset);
                         ctx->m_current_size += readSize;
@@ -397,7 +396,7 @@ namespace web { namespace http
                     }
 
                     auto readbuf = ctx->_get_readbuffer();
-                    size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size);
+                    const size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size);
 
                     readbuf.getn(boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(readSize)), readSize).then([=](pplx::task<size_t> op)
                     {
@@ -451,14 +450,14 @@ namespace web { namespace http
                             }
                         }
 
-                    // Read until the end of entire headers
-                    if (ctx->m_ssl_stream)
-                        boost::asio::async_read_until(*ctx->m_ssl_stream, ctx->m_body_buf, CRLF+CRLF,
-                            boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));
-                    else
-                        boost::asio::async_read_until(ctx->m_socket, ctx->m_body_buf, CRLF+CRLF,
-                            boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));
-                    }
+                        // Read until the end of entire headers
+                        if (ctx->m_ssl_stream)
+                            boost::asio::async_read_until(*ctx->m_ssl_stream, ctx->m_body_buf, CRLF+CRLF,
+                                boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));
+                        else
+                            boost::asio::async_read_until(ctx->m_socket, ctx->m_body_buf, CRLF+CRLF,
+                                boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error,  ctx));
+                        }
                     else
                     {
                         ctx->report_error("Failed to write request body", ec, httpclient_errorcode_context::writebody);
@@ -481,7 +480,7 @@ namespace web { namespace http
                         ctx->m_response.set_status_code(status_code);
 
                         trim_whitespace(status_message);
-                        ctx->m_response.set_reason_phrase(status_message);
+                        ctx->m_response.set_reason_phrase(std::move(status_message));
 
                         if (!response_stream || http_version.substr(0, 5) != "HTTP/")
                         {
@@ -504,20 +503,20 @@ namespace web { namespace http
                     std::string header;
                     while (std::getline(response_stream, header) && header != "\r")
                     {
-                        auto colon = header.find(':');
+                        const auto colon = header.find(':');
                         if (colon != std::string::npos)
                         {
                             auto name = header.substr(0, colon);
                             auto value = header.substr(colon+2, header.size()-(colon+3)); // also exclude '\r'
                             boost::algorithm::trim(name);
                             boost::algorithm::trim(value);
 
-                            ctx->m_response.headers().add(name, value);
-
                             if (boost::iequals(name, header_names::transfer_encoding))
                             {
                                 ctx->m_needChunked = boost::iequals(value, U("chunked"));
                             }
+
+                            ctx->m_response.headers().add(std::move(name), std::move(value));
                         }
                     }
                     ctx->complete_headers();
@@ -546,8 +545,10 @@ namespace web { namespace http
                     {
                         ctx->m_current_size = 0;
                         if (!ctx->m_needChunked)
+                        {
                             async_read_until_buffersize(std::min(ctx->m_known_size, client_config().chunksize()),
-                            boost::bind(&linux_client::handle_read_content, this, boost::asio::placeholders::error, ctx), ctx);
+                                boost::bind(&linux_client::handle_read_content, this, boost::asio::placeholders::error, ctx), ctx);
+                        }
                         else
                         {
                             if (ctx->m_ssl_stream)
@@ -567,13 +568,17 @@ namespace web { namespace http
                     if (ctx->m_ssl_stream)
                     {
                         if (ctx->m_body_buf.size() < size)
+                        {
                             size_to_read = size - ctx->m_body_buf.size();
+                        }
                         boost::asio::async_read(*ctx->m_ssl_stream, ctx->m_body_buf, boost::asio::transfer_at_least(size_to_read), handler);
                     }
                     else
                     {
                         if (ctx->m_body_buf.size() < size)
+                        {
                             size_to_read = size - ctx->m_body_buf.size();
+                        }
                         boost::asio::async_read(ctx->m_socket, ctx->m_body_buf, boost::asio::transfer_at_least(size_to_read), handler);
                     }
                 }
@@ -586,7 +591,7 @@ namespace web { namespace http
                         std::string line;
                         std::getline(response_stream, line);
 
-                        std::istringstream octetLine(line);
+                        std::istringstream octetLine(std::move(line));
                         int octets = 0;
                         octetLine >> std::hex >> octets;
 
@@ -625,7 +630,8 @@ namespace web { namespace http
                             ctx->m_body_buf.consume(CRLF.size());
                             ctx->_get_writebuffer().sync().then([ctx](pplx::task<void> op)
                             {
-                                try { 
+                                try 
+                                { 
                                     op.wait(); 
                                     ctx->complete_request(ctx->m_current_size);
                                 }

Original file line number	Diff line number	Diff line change
`@@ -52,8 +52,6 @@ namespace web { namespace http`
`52`	`52`	`close`
`53`	`53`	`};`
`54`	`54`
`55`		`- class linux_client;`
`56`		`-`
`57`	`55`	`class linux_request_context : public request_context`
`58`	`56`	`{`
`59`	`57`	`public:`
`@@ -127,6 +125,7 @@ namespace web { namespace http`
`127`	`125`	`}`
`128`	`126`
`129`	`127`	`linux_request_context(std::shared_ptr<_http_client_communicator> &client, http_request request);`
	`128`	`+`
`130`	`129`	`protected:`
`131`	`130`	`virtual void cleanup()`
`132`	`131`	`{`
`@@ -365,7 +364,7 @@ namespace web { namespace http`
`365`	`364`	`ctx->report_exception(std::current_exception());`
`366`	`365`	`return;`
`367`	`366`	`}`
`368`		`- size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space, readSize);`
	`367`	`+ const size_t offset = http::details::chunked_encoding::add_chunked_delimiters(buf, client_config().chunksize() + http::details::chunked_encoding::additional_encoding_space, readSize);`
`369`	`368`	`ctx->m_body_buf.commit(readSize + http::details::chunked_encoding::additional_encoding_space);`
`370`	`369`	`ctx->m_body_buf.consume(offset);`
`371`	`370`	`ctx->m_current_size += readSize;`
`@@ -397,7 +396,7 @@ namespace web { namespace http`
`397`	`396`	`}`
`398`	`397`
`399`	`398`	`auto readbuf = ctx->_get_readbuffer();`
`400`		`- size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size);`
	`399`	`+ const size_t readSize = std::min(client_config().chunksize(), ctx->m_known_size - ctx->m_current_size);`
`401`	`400`
`402`	`401`	`readbuf.getn(boost::asio::buffer_cast<uint8_t *>(ctx->m_body_buf.prepare(readSize)), readSize).then([=](pplx::task<size_t> op)`
`403`	`402`	`{`
`@@ -451,14 +450,14 @@ namespace web { namespace http`
`451`	`450`	`}`
`452`	`451`	`}`
`453`	`452`
`454`		`- // Read until the end of entire headers`
`455`		`- if (ctx->m_ssl_stream)`
`456`		`- boost::asio::async_read_until(*ctx->m_ssl_stream, ctx->m_body_buf, CRLF+CRLF,`
`457`		`- boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));`
`458`		`- else`
`459`		`- boost::asio::async_read_until(ctx->m_socket, ctx->m_body_buf, CRLF+CRLF,`
`460`		`- boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));`
`461`		`- }`
	`453`	`+ // Read until the end of entire headers`
	`454`	`+ if (ctx->m_ssl_stream)`
	`455`	`+ boost::asio::async_read_until(*ctx->m_ssl_stream, ctx->m_body_buf, CRLF+CRLF,`
	`456`	`+ boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));`
	`457`	`+ else`
	`458`	`+ boost::asio::async_read_until(ctx->m_socket, ctx->m_body_buf, CRLF+CRLF,`
	`459`	`+ boost::bind(&linux_client::handle_status_line, this, boost::asio::placeholders::error, ctx));`
	`460`	`+ }`
`462`	`461`	`else`
`463`	`462`	`{`
`464`	`463`	`ctx->report_error("Failed to write request body", ec, httpclient_errorcode_context::writebody);`
`@@ -481,7 +480,7 @@ namespace web { namespace http`
`481`	`480`	`ctx->m_response.set_status_code(status_code);`
`482`	`481`
`483`	`482`	`trim_whitespace(status_message);`
`484`		`- ctx->m_response.set_reason_phrase(status_message);`
	`483`	`+ ctx->m_response.set_reason_phrase(std::move(status_message));`
`485`	`484`
`486`	`485`	`if (!response_stream \|\| http_version.substr(0, 5) != "HTTP/")`
`487`	`486`	`{`
`@@ -504,20 +503,20 @@ namespace web { namespace http`
`504`	`503`	`std::string header;`
`505`	`504`	`while (std::getline(response_stream, header) && header != "\r")`
`506`	`505`	`{`
`507`		`- auto colon = header.find(':');`
	`506`	`+ const auto colon = header.find(':');`
`508`	`507`	`if (colon != std::string::npos)`
`509`	`508`	`{`
`510`	`509`	`auto name = header.substr(0, colon);`
`511`	`510`	`auto value = header.substr(colon+2, header.size()-(colon+3)); // also exclude '\r'`
`512`	`511`	`boost::algorithm::trim(name);`
`513`	`512`	`boost::algorithm::trim(value);`
`514`	`513`
`515`		`- ctx->m_response.headers().add(name, value);`
`516`		`-`
`517`	`514`	`if (boost::iequals(name, header_names::transfer_encoding))`
`518`	`515`	`{`
`519`	`516`	`ctx->m_needChunked = boost::iequals(value, U("chunked"));`
`520`	`517`	`}`
	`518`	`+`
	`519`	`+ ctx->m_response.headers().add(std::move(name), std::move(value));`
`521`	`520`	`}`
`522`	`521`	`}`
`523`	`522`	`ctx->complete_headers();`
`@@ -546,8 +545,10 @@ namespace web { namespace http`
`546`	`545`	`{`
`547`	`546`	`ctx->m_current_size = 0;`
`548`	`547`	`if (!ctx->m_needChunked)`
	`548`	`+ {`
`549`	`549`	`async_read_until_buffersize(std::min(ctx->m_known_size, client_config().chunksize()),`
`550`		`- boost::bind(&linux_client::handle_read_content, this, boost::asio::placeholders::error, ctx), ctx);`
	`550`	`+ boost::bind(&linux_client::handle_read_content, this, boost::asio::placeholders::error, ctx), ctx);`
	`551`	`+ }`
`551`	`552`	`else`
`552`	`553`	`{`
`553`	`554`	`if (ctx->m_ssl_stream)`
`@@ -567,13 +568,17 @@ namespace web { namespace http`
`567`	`568`	`if (ctx->m_ssl_stream)`
`568`	`569`	`{`
`569`	`570`	`if (ctx->m_body_buf.size() < size)`
	`571`	`+ {`
`570`	`572`	`size_to_read = size - ctx->m_body_buf.size();`
	`573`	`+ }`
`571`	`574`	`boost::asio::async_read(*ctx->m_ssl_stream, ctx->m_body_buf, boost::asio::transfer_at_least(size_to_read), handler);`
`572`	`575`	`}`
`573`	`576`	`else`
`574`	`577`	`{`
`575`	`578`	`if (ctx->m_body_buf.size() < size)`
	`579`	`+ {`
`576`	`580`	`size_to_read = size - ctx->m_body_buf.size();`
	`581`	`+ }`
`577`	`582`	`boost::asio::async_read(ctx->m_socket, ctx->m_body_buf, boost::asio::transfer_at_least(size_to_read), handler);`
`578`	`583`	`}`
`579`	`584`	`}`
`@@ -586,7 +591,7 @@ namespace web { namespace http`
`586`	`591`	`std::string line;`
`587`	`592`	`std::getline(response_stream, line);`
`588`	`593`
`589`		`- std::istringstream octetLine(line);`
	`594`	`+ std::istringstream octetLine(std::move(line));`
`590`	`595`	`int octets = 0;`
`591`	`596`	`octetLine >> std::hex >> octets;`
`592`	`597`
`@@ -625,7 +630,8 @@ namespace web { namespace http`
`625`	`630`	`ctx->m_body_buf.consume(CRLF.size());`
`626`	`631`	`ctx->_get_writebuffer().sync().then([ctx](pplx::task<void> op)`
`627`	`632`	`{`
`628`		`- try {`
	`633`	`+ try`
	`634`	`+ {`
`629`	`635`	`op.wait();`
`630`	`636`	`ctx->complete_request(ctx->m_current_size);`
`631`	`637`	`}`