This is an automated email from the ASF dual-hosted git repository.
maskit 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 5c3352eb5b xpack: Fix division by zero error (#11107)
5c3352eb5b is described below
commit 5c3352eb5bbc3f11a11bff663b3befa956cf7ddb
Author: Masakazu Kitajo <[email protected]>
AuthorDate: Wed Feb 28 11:21:06 2024 -0700
xpack: Fix division by zero error (#11107)
* xpack: Fix division by zero error
* Simplify a formula
---
include/proxy/hdrs/XPACK.h | 5 +++++
src/proxy/hdrs/XPACK.cc | 32 +++++++++++++++++++++-----------
src/proxy/hdrs/unit_tests/test_XPACK.cc | 13 +++++++++++++
3 files changed, 39 insertions(+), 11 deletions(-)
diff --git a/include/proxy/hdrs/XPACK.h b/include/proxy/hdrs/XPACK.h
index f4b705a761..99829fe0cb 100644
--- a/include/proxy/hdrs/XPACK.h
+++ b/include/proxy/hdrs/XPACK.h
@@ -112,4 +112,9 @@ private:
* Passing a value more than UINT32_MAX evicts every entry and return false.
*/
bool _make_space(uint64_t required_size);
+
+ /**
+ * Calcurates the index number for _entries, which is a kind of circular
buffer.
+ */
+ uint32_t _calc_index(uint32_t base, int64_t offset) const;
};
diff --git a/src/proxy/hdrs/XPACK.cc b/src/proxy/hdrs/XPACK.cc
index e5eb070bd5..ca7d39e425 100644
--- a/src/proxy/hdrs/XPACK.cc
+++ b/src/proxy/hdrs/XPACK.cc
@@ -245,12 +245,12 @@ XpackDynamicTable::lookup(uint32_t index, const char
**name, size_t *name_len, c
return {0, XpackLookupResult::MatchType::NONE};
}
- if (index < this->_entries[(this->_entries_tail + 1) %
this->_max_entries].index) {
+ if (index < this->_entries[this->_calc_index(this->_entries_tail, 1)].index)
{
// The index is invalid
return {0, XpackLookupResult::MatchType::NONE};
}
- uint32_t pos = (this->_entries_head -
(this->_entries[this->_entries_head].index - index)) % this->_max_entries;
+ uint32_t pos = this->_calc_index(this->_entries_head, index -
this->_entries[this->_entries_head].index);
*name_len = this->_entries[pos].name_len;
*value_len = this->_entries[pos].value_len;
this->_storage.read(this->_entries[pos].offset, name, *name_len, value,
*value_len);
@@ -265,8 +265,8 @@ XpackDynamicTable::lookup(const char *name, size_t
name_len, const char *value,
{
XPACKDebug("Lookup entry: name=%.*s, value=%.*s",
static_cast<int>(name_len), name, static_cast<int>(value_len), value);
XpackLookupResult::MatchType match_type = XpackLookupResult::MatchType::NONE;
- uint32_t i = (this->_entries_tail + 1) %
this->_max_entries;
- uint32_t end = (this->_entries_head + 1) %
this->_max_entries;
+ uint32_t i =
this->_calc_index(this->_entries_tail, 1);
+ uint32_t end =
this->_calc_index(this->_entries_head, 1);
uint32_t candidate_index = 0;
const char *tmp_name = nullptr;
const char *tmp_value = nullptr;
@@ -276,7 +276,7 @@ XpackDynamicTable::lookup(const char *name, size_t
name_len, const char *value,
return {candidate_index, match_type};
}
- for (; i != end; i = (i + 1) % this->_max_entries) {
+ for (; i != end; i = this->_calc_index(i, 1)) {
if (name_len != 0 && this->_entries[i].name_len == name_len) {
this->_storage.read(this->_entries[i].offset, &tmp_name,
this->_entries[i].name_len, &tmp_value, this->_entries[i].value_len);
if (match(name, name_len, tmp_name, this->_entries[i].name_len)) {
@@ -345,7 +345,7 @@ XpackDynamicTable::insert_entry(const char *name, size_t
name_len, const char *v
// Insert
const char *wks = nullptr;
hdrtoken_tokenize(name, name_len, &wks);
- this->_entries_head = (this->_entries_head + 1) %
this->_max_entries;
+ this->_entries_head = this->_calc_index(this->_entries_head,
1);
this->_entries[this->_entries_head] = {
this->_entries_inserted++,
this->_storage.write(name, static_cast<uint32_t>(name_len), value,
static_cast<uint32_t>(value_len)),
@@ -433,14 +433,14 @@ XpackDynamicTable::maximum_size() const
void
XpackDynamicTable::ref_entry(uint32_t index)
{
- uint32_t pos = (this->_entries_head + (index -
this->_entries[this->_entries_head].index)) % this->_max_entries;
+ uint32_t pos = this->_calc_index(this->_entries_head, (index -
this->_entries[this->_entries_head].index));
++this->_entries[pos].ref_count;
}
void
XpackDynamicTable::unref_entry(uint32_t index)
{
- uint32_t pos = (this->_entries_head + (index -
this->_entries[this->_entries_head].index)) % this->_max_entries;
+ uint32_t pos = this->_calc_index(this->_entries_head, (index -
this->_entries[this->_entries_head].index));
--this->_entries[pos].ref_count;
}
@@ -474,7 +474,7 @@ bool
XpackDynamicTable::_make_space(uint64_t required_size)
{
uint32_t freed = 0;
- uint32_t tail = (this->_entries_tail + 1) % this->_max_entries;
+ uint32_t tail = this->_calc_index(this->_entries_tail, 1);
while (required_size > freed) {
if (this->_entries_head < tail) {
@@ -484,12 +484,12 @@ XpackDynamicTable::_make_space(uint64_t required_size)
break;
}
freed += this->_entries[tail].name_len + this->_entries[tail].value_len +
ADDITIONAL_32_BYTES;
- tail = (tail + 1) % this->_max_entries;
+ tail = this->_calc_index(tail, 1);
}
// Evict
if (freed > 0) {
- XPACKDebug("Evict entries: from %u to %u",
this->_entries[(this->_entries_tail + 1) % this->_max_entries].index,
+ XPACKDebug("Evict entries: from %u to %u",
this->_entries[this->_calc_index(this->_entries_tail, 1)].index,
this->_entries[tail - 1].index);
this->_available += freed;
this->_entries_tail = tail - 1;
@@ -499,6 +499,16 @@ XpackDynamicTable::_make_space(uint64_t required_size)
return required_size <= this->_available;
}
+uint32_t
+XpackDynamicTable::_calc_index(uint32_t base, int64_t offset) const
+{
+ if (unlikely(this->_max_entries == 0)) {
+ return base + offset;
+ } else {
+ return (base + offset) % this->_max_entries;
+ }
+}
+
//
// DynamicTableStorage
//
diff --git a/src/proxy/hdrs/unit_tests/test_XPACK.cc
b/src/proxy/hdrs/unit_tests/test_XPACK.cc
index 222289c2f3..bfcc2fae3e 100644
--- a/src/proxy/hdrs/unit_tests/test_XPACK.cc
+++ b/src/proxy/hdrs/unit_tests/test_XPACK.cc
@@ -126,6 +126,19 @@ TEST_CASE("XPACK_String", "[xpack]")
}
}
+ SECTION("Zero-size Dynamic Table")
+ {
+ XpackDynamicTable dt(0);
+ XpackLookupResult result;
+
+ REQUIRE(dt.size() == 0);
+ REQUIRE(dt.maximum_size() == 0);
+ REQUIRE(dt.is_empty());
+ REQUIRE(dt.count() == 0);
+ result = dt.lookup("", "");
+ REQUIRE(result.match_type == XpackLookupResult::MatchType::NONE);
+ }
+
SECTION("Dynamic Table")
{
constexpr uint16_t MAX_SIZE = 128;
