This is an automated email from the ASF dual-hosted git repository. masaori pushed a commit to branch master in repository https://gitbox.apache.org/repos/asf/trafficserver.git
The following commit(s) were added to refs/heads/master by this push: new 33097a8 cppcheck: Fix various issues in proxy/http2/ 33097a8 is described below commit 33097a8d385b933811107db8775c6856fb57f56d Author: Masaori Koshiba <masaori...@gmail.com> AuthorDate: Thu Apr 25 17:50:48 2019 +0800 cppcheck: Fix various issues in proxy/http2/ --- proxy/http2/HPACK.cc | 4 ++-- proxy/http2/HPACK.h | 13 +++++++++-- proxy/http2/Http2Stream.cc | 7 +++--- proxy/http2/Http2Stream.h | 3 ++- proxy/http2/HuffmanCodec.cc | 9 ++++---- proxy/http2/test_HPACK.cc | 26 ++++++++++++---------- proxy/http2/unit_tests/test_Http2DependencyTree.cc | 6 ++--- 7 files changed, 40 insertions(+), 28 deletions(-) diff --git a/proxy/http2/HPACK.cc b/proxy/http2/HPACK.cc index 1c45d1e..5abe3de 100644 --- a/proxy/http2/HPACK.cc +++ b/proxy/http2/HPACK.cc @@ -385,7 +385,7 @@ bool HpackDynamicTable::update_maximum_size(uint32_t new_size) { while (_current_size > new_size) { - if (_headers.size() <= 0) { + if (_headers.size() == 0) { return false; } int last_name_len, last_value_len; @@ -450,7 +450,7 @@ encode_string(uint8_t *buf_start, const uint8_t *buf_end, const char *value, siz int64_t data_len = 0; // TODO Choose whether to use Huffman encoding wisely - + // cppcheck-suppress knownConditionTrueFalse; leaving "use_huffman" for wise huffman usage in the future if (use_huffman && value_len) { data = static_cast<char *>(ats_malloc(value_len * 4)); if (data == nullptr) { diff --git a/proxy/http2/HPACK.h b/proxy/http2/HPACK.h index 9f93d4e..78efb29 100644 --- a/proxy/http2/HPACK.h +++ b/proxy/http2/HPACK.h @@ -105,7 +105,7 @@ private: class HpackDynamicTable { public: - HpackDynamicTable(uint32_t size) : _current_size(0), _maximum_size(size) + explicit HpackDynamicTable(uint32_t size) : _current_size(0), _maximum_size(size) { _mhdr = new MIMEHdr(); _mhdr->create(); @@ -119,6 +119,10 @@ public: delete _mhdr; } + // noncopyable + HpackDynamicTable(HpackDynamicTable &) = delete; + HpackDynamicTable &operator=(const HpackDynamicTable &) = delete; + const MIMEField *get_header_field(uint32_t index) const; void add_header_field(const MIMEField *field); @@ -140,8 +144,13 @@ private: class HpackIndexingTable { public: - HpackIndexingTable(uint32_t size) { _dynamic_table = new HpackDynamicTable(size); } + explicit HpackIndexingTable(uint32_t size) { _dynamic_table = new HpackDynamicTable(size); } ~HpackIndexingTable() { delete _dynamic_table; } + + // noncopyable + HpackIndexingTable(HpackIndexingTable &) = delete; + HpackIndexingTable &operator=(const HpackIndexingTable &) = delete; + HpackLookupResult lookup(const MIMEFieldWrapper &field) const; HpackLookupResult lookup(const char *name, int name_len, const char *value, int value_len) const; int get_header_field(uint32_t index, MIMEFieldWrapper &header_field) const; diff --git a/proxy/http2/Http2Stream.cc b/proxy/http2/Http2Stream.cc index c6a83ed..46614a6 100644 --- a/proxy/http2/Http2Stream.cc +++ b/proxy/http2/Http2Stream.cc @@ -154,11 +154,10 @@ Http2Stream::send_request(Http2ConnectionState &cstate) int bufindex; int dumpoffset = 0; int done, tmp; - IOBufferBlock *block; do { - bufindex = 0; - tmp = dumpoffset; - block = request_buffer.get_current_block(); + bufindex = 0; + tmp = dumpoffset; + IOBufferBlock *block = request_buffer.get_current_block(); if (!block) { request_buffer.add_block(); block = request_buffer.get_current_block(); diff --git a/proxy/http2/Http2Stream.h b/proxy/http2/Http2Stream.h index a753d0d..71fc00d 100644 --- a/proxy/http2/Http2Stream.h +++ b/proxy/http2/Http2Stream.h @@ -41,6 +41,8 @@ public: typedef ProxyClientTransaction super; ///< Parent type. Http2Stream(Http2StreamId sid = 0, ssize_t initial_rwnd = Http2::initial_window_size) : client_rwnd(initial_rwnd), _id(sid) { + http_parser_init(&http_parser); + SET_HANDLER(&Http2Stream::main_event_handler); } @@ -52,7 +54,6 @@ public: _thread = this_ethread(); this->client_rwnd = initial_rwnd; sm_reader = request_reader = request_buffer.alloc_reader(); - http_parser_init(&http_parser); // FIXME: Are you sure? every "stream" needs request_header? _req_header.create(HTTP_TYPE_REQUEST); response_header.create(HTTP_TYPE_RESPONSE); diff --git a/proxy/http2/HuffmanCodec.cc b/proxy/http2/HuffmanCodec.cc index 0c10373..562fa36 100644 --- a/proxy/http2/HuffmanCodec.cc +++ b/proxy/http2/HuffmanCodec.cc @@ -93,12 +93,12 @@ static Node * make_huffman_tree() { Node *root = make_huffman_tree_node(); - Node *current; - uint32_t bit_len; + // insert leafs for each ascii code for (unsigned i = 0; i < countof(huffman_table); i++) { - bit_len = huffman_table[i].bit_len; - current = root; + uint32_t bit_len = huffman_table[i].bit_len; + Node *current = root; + while (bit_len > 0) { if (huffman_table[i].code_as_hex & (1 << (bit_len - 1))) { if (!current->right) { @@ -116,6 +116,7 @@ make_huffman_tree() current->ascii_code = i; current->leaf_node = true; } + return root; } diff --git a/proxy/http2/test_HPACK.cc b/proxy/http2/test_HPACK.cc index b1d4ed4..be29e8f 100644 --- a/proxy/http2/test_HPACK.cc +++ b/proxy/http2/test_HPACK.cc @@ -67,10 +67,9 @@ int unpack(string &packed, uint8_t *unpacked) { int n = packed.length() / 2; - int u, l; for (int i = 0; i < n; ++i) { - u = packed[i * 2]; - l = packed[i * 2 + 1]; + int u = packed[i * 2]; + int l = packed[i * 2 + 1]; unpacked[i] = (((u >= 'a') ? u - 'a' + 10 : u - '0') << 4) + ((l >= 'a') ? l - 'a' + 10 : l - '0'); } return n; @@ -129,21 +128,21 @@ print_difference(const char *a_str, const int a_str_len, const char *b_str, cons int compare_header_fields(HTTPHdr *a, HTTPHdr *b) { - const char *a_str, *b_str; - int a_str_len, b_str_len; + // compare fields count + if (a->fields_count() != b->fields_count()) { + return -1; + } + MIMEFieldIter a_iter, b_iter; const MIMEField *a_field = a->iter_get_first(&a_iter); const MIMEField *b_field = b->iter_get_first(&b_iter); - // compare fields count - if (a->fields_count() != b->fields_count()) { - return -1; - } - for (; a_field != nullptr; a_field = a->iter_get_next(&a_iter), b_field = b->iter_get_next(&b_iter)) { + while (a_field != nullptr && b_field != nullptr) { + int a_str_len, b_str_len; // compare header name - a_str = a_field->name_get(&a_str_len); - b_str = b_field->name_get(&b_str_len); + const char *a_str = a_field->name_get(&a_str_len); + const char *b_str = b_field->name_get(&b_str_len); if (a_str_len != b_str_len) { if (memcmp(a_str, b_str, a_str_len) != 0) { print_difference(a_str, a_str_len, b_str, b_str_len); @@ -159,6 +158,9 @@ compare_header_fields(HTTPHdr *a, HTTPHdr *b) return -1; } } + + a_field = a->iter_get_next(&a_iter); + b_field = b->iter_get_next(&b_iter); } return 0; diff --git a/proxy/http2/unit_tests/test_Http2DependencyTree.cc b/proxy/http2/unit_tests/test_Http2DependencyTree.cc index b72bbce..6a00b1a 100644 --- a/proxy/http2/unit_tests/test_Http2DependencyTree.cc +++ b/proxy/http2/unit_tests/test_Http2DependencyTree.cc @@ -282,7 +282,7 @@ TEST_CASE("Http2DependencyTree_Chrome_50", "[http2][Http2DependencyTree]") ostringstream oss; - for (int i = 0; i < 108; ++i) { + for (int index = 0; index < 108; ++index) { Node *node = tree->top(); oss << static_cast<string *>(node->t)->c_str(); @@ -336,7 +336,7 @@ TEST_CASE("Http2DependencyTree_Chrome_51", "[http2][Http2DependencyTree]") ostringstream oss; - for (int i = 0; i < 9; ++i) { + for (int index = 0; index < 9; ++index) { Node *node = tree->top(); if (node != nullptr) { oss << static_cast<string *>(node->t)->c_str(); @@ -352,7 +352,7 @@ TEST_CASE("Http2DependencyTree_Chrome_51", "[http2][Http2DependencyTree]") tree->activate(node_f); tree->activate(node_h); - for (int i = 0; i < 9; ++i) { + for (int index = 0; index < 9; ++index) { Node *node = tree->top(); if (node != nullptr) { oss << static_cast<string *>(node->t)->c_str();