Move parts of interface not needing to be public to private.

Avoid complicated buffer management from the time when malloc was expensive.

2 files changed, 50 insertions, 74 deletions

diff --git a/fbench/src/httpclient/httpclient.cpp b/fbench/src/httpclient/httpclient.cpp
index 9265905c9f6..2feed087b43 100644
--- a/fbench/src/httpclient/httpclient.cpp
+++ b/fbench/src/httpclient/httpclient.cpp
@@ -110,8 +110,7 @@ HTTPClient::ReadLine(char *buf, size_t bufsize)
 bool
 HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen)
 {
-    char tmp[4096];
-    char *req = NULL;
+    std::unique_ptr<char[]> req;
     uint32_t req_max = 0;
     uint32_t url_len = strlen(url);
     uint32_t host_len = _hostname.size();
@@ -126,14 +125,8 @@ HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen
         headers += "X-Yahoo-Vespa-Benchmarkdata-Coverage: true\r\n";
     }
 
-    if (url_len + host_len + headers.length() + FIXED_REQ_MAX < sizeof(tmp)) {
-        req = tmp;
-        req_max = sizeof(tmp);
-    } else {
-        req_max = url_len + host_len + headers.length() + FIXED_REQ_MAX;
-        req = new char[req_max];
-        assert(req != NULL);
-    }
+    req_max = url_len + host_len + headers.length() + FIXED_REQ_MAX;
+    req = std::make_unique<char []>(req_max);
 
     if (!_keepAlive) {
         headers += "Connection: close\r\n";
@@ -142,7 +135,7 @@ HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen
 
     // create request
     if (usePost) {
-        snprintf(req, req_max,
+        snprintf(req.get(), req_max,
                  "POST %s HTTP/1.1\r\n"
                  "Host: %s\r\n"
                  "Content-Length: %d\r\n"
@@ -150,7 +143,7 @@ HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen
                  "\r\n",
                  url, _host_header_value.c_str(), cLen, headers.c_str());
     } else {
-        snprintf(req, req_max,
+        snprintf(req.get(), req_max,
                  "GET %s HTTP/1.1\r\n"
                  "Host: %s\r\n"
                  "%s"
@@ -159,18 +152,16 @@ HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen
     }
 
     // try to reuse connection if keep-alive is enabled
+    ssize_t reqLen = strlen(req.get());
     if (_keepAlive
         && _socket
-        && _socket->write(req, strlen(req)) == (ssize_t)strlen(req)
+        && _socket->write(req.get(), reqLen) == reqLen
         && (!usePost || _socket->write(content, cLen) == (ssize_t)cLen)
         && FillBuffer() > 0) {
 
         // DEBUG
         // printf("Socket Connection reused!\n");
         _reuseCount++;
-        if (req != tmp) {
-            delete [] req;
-        }
         return true;
     } else {
         _socket.reset();
@@ -179,25 +170,14 @@ HTTPClient::Connect(const char *url, bool usePost, const char *content, int cLen
 
     // try to open new connection to server
     if (connect_socket()
-        && _socket->write(req, strlen(req)) == (ssize_t)strlen(req)
+        && _socket->write(req.get(), reqLen) == reqLen
         && (!usePost || _socket->write(content, cLen) == (ssize_t)cLen))
     {
-
-        // DEBUG
-        // printf("New Socket connection!\n");
-        if (req != tmp) {
-            delete [] req;
-        }
         return true;
     } else {
         _socket.reset();
     }
 
-    // DEBUG
-    // printf("Connect FAILED!\n");
-    if (req != tmp) {
-        delete [] req;
-    }
     return false;
 }
 
@@ -214,7 +194,7 @@ HTTPClient::SplitString(char *input, int &argc, char **argv, int maxargs)
         }
     if (*(argv[argc]) != '\0')
         argc++;
-    return NULL;                    // COMPLETE
+    return nullptr;                    // COMPLETE
 }
 
 bool
@@ -241,8 +221,7 @@ HTTPClient::ReadHTTPHeader(std::string & headerinfo)
     if (argc >= 2) {
         if (strncmp(argv[0], "HTTP/", 5) != 0)
             return false;
-        _httpVersion = (strncmp(argv[0], "HTTP/1.0", 8) == 0) ?
-                       0 : 1;
+        _httpVersion = (strncmp(argv[0], "HTTP/1.0", 8) == 0) ? 0 : 1;
         _requestStatus = atoi(argv[1]);
     } else {
         return false;
@@ -319,7 +298,7 @@ HTTPClient::ReadChunkHeader()
     char c;
     int  i;
 
-    if (_chunkSeq++ > 0 && ReadLine(NULL, 0) != 0)
+    if (_chunkSeq++ > 0 && ReadLine(nullptr, 0) != 0)
         return false;                  // no CRLF(/LF) after data block
 
     assert(_chunkLeft == 0);
@@ -344,7 +323,7 @@ HTTPClient::ReadChunkHeader()
     // printf("CHUNK: Length: %d\n", _chunkLeft);
 
     if (_chunkLeft == 0) {
-        while ((lineLen = ReadLine(NULL, 0)) > 0);   // skip trailer
+        while ((lineLen = ReadLine(nullptr, 0)) > 0);   // skip trailer
         if (lineLen < 0)
             return false;                              // data error
         _dataDone = true;                            // got last chunk
@@ -548,7 +527,7 @@ HTTPClient::Fetch(const char *url, std::ostream *file,
     }
 
     while((readRes = Read(buf, buflen)) > 0) {
-        if(file != NULL) {
+        if(file != nullptr) {
             if (!file->write(buf, readRes)) {
                 Close();
                 return FetchStatus(false, _requestStatus, _totalHitCount, written);
diff --git a/fbench/src/httpclient/httpclient.h b/fbench/src/httpclient/httpclient.h
index 31a3881ec27..5b96daa2441 100644
--- a/fbench/src/httpclient/httpclient.h
+++ b/fbench/src/httpclient/httpclient.h
@@ -109,8 +109,6 @@ protected:
   ssize_t          _bufused;
   ssize_t          _bufpos;
 
-  unsigned int     _headerinfoPos;
-
   bool             _isOpen;
   unsigned int     _httpVersion;
   unsigned int     _requestStatus;
@@ -221,37 +219,6 @@ protected:
    **/
   bool ReadChunkHeader();
 
-public:
-
-  /**
-   * Create a HTTP client that may be used to fetch documents from the
-   * given host.
-   *
-   * @param hostname the host you want to fetch documents from.
-   * @param port the TCP port to use when contacting the host.
-   * @param keepAlive flag indicating if keep-alive should be enabled.
-   **/
-    HTTPClient(vespalib::CryptoEngine::SP engine, const char *hostname, int port, bool keepAlive,
-               bool headerBenchmarkdataCoverage, const std::string & extraHeaders="", const std::string &authority = "");
-
-  /**
-   * Disconnect from server and free memory.
-   **/
-  ~HTTPClient();
-
-  /**
-   * This method may be used to obtain information about how many
-   * times a physical connection has been reused to send an additional
-   * HTTP request. Note that connections may only be reused if
-   * keep-alive is enabled.
-   *
-   * @return connection reuse count
-   **/
-  uint64_t GetReuseCount() const
-  {
-    return _reuseCount;
-  }
-
   /**
    * Connect to the given url and read the response HTTP header. Note
    * that this method will fail if the host returns a status code
@@ -265,6 +232,17 @@ public:
    * @param cLen length of content in bytes
    **/
   bool Open(std::string & headerinfo, const char *url, bool usePost = false, const char *content = 0, int cLen = 0);
+  
+  /**
+   * Close the connection to the url we are currently reading
+   * from. Will also close the physical connection if keepAlive is not
+   * enabled or if all the url content was not read. This is done
+   * because skipping will probably be more expencive than creating a
+   * new connection.
+   *
+   * @return success(true)/failure(false)
+   **/
+  bool Close();
 
   /**
    * Read data from the url we are currently connected to. This method
@@ -278,17 +256,36 @@ public:
    * @param len length of buf.
    **/
   ssize_t Read(void *buf, size_t len);
+public:
 
   /**
-   * Close the connection to the url we are currently reading
-   * from. Will also close the physical connection if keepAlive is not
-   * enabled or if all the url content was not read. This is done
-   * because skipping will probably be more expencive than creating a
-   * new connection.
+   * Create a HTTP client that may be used to fetch documents from the
+   * given host.
    *
-   * @return success(true)/failure(false)
+   * @param hostname the host you want to fetch documents from.
+   * @param port the TCP port to use when contacting the host.
+   * @param keepAlive flag indicating if keep-alive should be enabled.
    **/
-  bool Close();
+    HTTPClient(vespalib::CryptoEngine::SP engine, const char *hostname, int port, bool keepAlive,
+               bool headerBenchmarkdataCoverage, const std::string & extraHeaders="", const std::string &authority = "");
+
+  /**
+   * Disconnect from server and free memory.
+   **/
+  ~HTTPClient();
+
+  /**
+   * This method may be used to obtain information about how many
+   * times a physical connection has been reused to send an additional
+   * HTTP request. Note that connections may only be reused if
+   * keep-alive is enabled.
+   *
+   * @return connection reuse count
+   **/
+  uint64_t GetReuseCount() const
+  {
+    return _reuseCount;
+  }
 
   /**
    * Class that provides status about the executed fetch method.