You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@trafficserver.apache.org by ma...@apache.org on 2019/04/27 12:18:52 UTC
[trafficserver] branch master updated: cppcheck: Fix various issues
in proxy/http2/
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 <ma...@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();