This is an automated email from the ASF dual-hosted git repository.
bneradt 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 b3b7a5674c Fix XpackDynamicTable::_make_space return value (#11408)
b3b7a5674c is described below
commit b3b7a5674cd5c59c3470ec0597e43762ebaef60b
Author: Brian Neradt <[email protected]>
AuthorDate: Fri May 31 17:18:20 2024 -0500
Fix XpackDynamicTable::_make_space return value (#11408)
Callers expect `_make_space` to return true if it was able to free the
amount of given space, but it rather returns true if the amount of space
available exceeds the passed in value. This fixes the function to return
the intended result.
---
include/proxy/hdrs/XPACK.h | 12 ++++++++----
src/proxy/hdrs/XPACK.cc | 10 +++++-----
src/proxy/hdrs/unit_tests/test_XPACK.cc | 4 ++--
3 files changed, 15 insertions(+), 11 deletions(-)
diff --git a/include/proxy/hdrs/XPACK.h b/include/proxy/hdrs/XPACK.h
index 685dc6e7f7..da41b069f9 100644
--- a/include/proxy/hdrs/XPACK.h
+++ b/include/proxy/hdrs/XPACK.h
@@ -235,11 +235,15 @@ private:
*/
void _expand_storage_size(uint32_t new_storage_size);
- /**
- * The type of reuired_size is uint64 so that we can handle a size that is
begger than the table capacity.
- * Passing a value more than UINT32_MAX evicts every entry and return false.
+ /** Evict entries to obtain the extra space needed.
+ *
+ * The type of reuired_size is uint64 so that we can handle a size that is
bigger than the table capacity.
+ * Passing a value more than UINT32_MAX evicts every entry and returns false.
+ *
+ * @param[in] extra_space_needed The amount of space needed to be freed.
+ * @return true if the required space was freed, false otherwise.
*/
- bool _make_space(uint64_t required_size);
+ bool _make_space(uint64_t extra_space_needed);
/** Calcurates the index number for _entries, which is a kind of circular
buffer.
*
diff --git a/src/proxy/hdrs/XPACK.cc b/src/proxy/hdrs/XPACK.cc
index 9eb83dde32..c910526909 100644
--- a/src/proxy/hdrs/XPACK.cc
+++ b/src/proxy/hdrs/XPACK.cc
@@ -494,16 +494,16 @@ XpackDynamicTable::_expand_storage_size(uint32_t
new_storage_size)
}
bool
-XpackDynamicTable::_make_space(uint64_t required_size)
+XpackDynamicTable::_make_space(uint64_t extra_space_needed)
{
if (is_empty()) {
- // if the table is empty, skip and just check if there is enough space
- return required_size <= this->_available;
+ // If the table is empty, there's nothing to free.
+ return extra_space_needed == 0;
}
uint32_t freed = 0;
uint32_t tail = this->_calc_index(this->_entries_tail, 1);
- while (required_size > freed) {
+ while (extra_space_needed > freed) {
if (this->_entries_head < tail) {
break;
}
@@ -523,7 +523,7 @@ XpackDynamicTable::_make_space(uint64_t required_size)
XPACKDbg("Available size: %u", this->_available);
}
- return required_size <= this->_available;
+ return freed >= extra_space_needed;
}
uint32_t
diff --git a/src/proxy/hdrs/unit_tests/test_XPACK.cc
b/src/proxy/hdrs/unit_tests/test_XPACK.cc
index 8e8258bc5b..93fc1bb1f4 100644
--- a/src/proxy/hdrs/unit_tests/test_XPACK.cc
+++ b/src/proxy/hdrs/unit_tests/test_XPACK.cc
@@ -275,8 +275,8 @@ TEST_CASE("XPACK_String", "[xpack]")
REQUIRE(memcmp(value, "value2", value_len) == 0);
// Insert one more entry (this should evict all existing entries)
- std::string field_4 = get_long_string(50);
- dt.insert_entry(field_4, field_4); // 100 bytes. _head should now be 0.
+ std::string field_4 = get_long_string(40);
+ dt.insert_entry(field_4, field_4); // 80 bytes. _head should now be 0.
REQUIRE(dt.size() == 2 * field_4.length() + 32);
REQUIRE(dt.maximum_size() == MAX_SIZE);
REQUIRE(dt.count() == 1);
