From aeb9251d33b5326df1dbe7348a600012a3e959a2 Mon Sep 17 00:00:00 2001 From: James Buckland Date: Thu, 12 Jul 2018 11:12:23 -0400 Subject: [PATCH] Refactor sizeGivenName(), sizeGivenStatsOptions() into structSize(), structSizeWithOptions() Signed-off-by: James Buckland --- source/common/common/block_memory_hash_set.h | 2 +- source/common/stats/stats_impl.cc | 12 ++++++------ source/common/stats/stats_impl.h | 11 +++++------ source/server/hot_restart_impl.cc | 4 ++-- test/common/common/block_memory_hash_set_test.cc | 2 +- test/common/stats/stats_impl_test.cc | 2 +- test/test_common/utility.h | 2 +- 7 files changed, 17 insertions(+), 18 deletions(-) diff --git a/source/common/common/block_memory_hash_set.h b/source/common/common/block_memory_hash_set.h index e01613a075dd..e28a1c59595e 100644 --- a/source/common/common/block_memory_hash_set.h +++ b/source/common/common/block_memory_hash_set.h @@ -330,7 +330,7 @@ template class BlockMemoryHashSet : public Logger::Loggable } RawStatData* HeapRawStatDataAllocator::alloc(const std::string& name) { - uint64_t num_bytes_to_allocate = RawStatData::sizeGivenName(name); + uint64_t num_bytes_to_allocate = RawStatData::structSize(name.size()); RawStatData* data = static_cast(::calloc(num_bytes_to_allocate, 1)); if (data == nullptr) { throw EnvoyException("HeapRawStatDataAllocator: unable to allocate a new stat"); @@ -344,7 +344,7 @@ void RawStatData::initialize(absl::string_view key, uint64_t xfer_size) { void RawStatData::checkAndInit(absl::string_view key, uint64_t num_bytes_allocated) { uint64_t xfer_size = key.size(); - ASSERT(sizeof(RawStatData) + xfer_size + 1 <= num_bytes_allocated); + ASSERT(structSize(xfer_size) <= num_bytes_allocated); initialize(key, xfer_size); } diff --git a/source/common/stats/stats_impl.h b/source/common/stats/stats_impl.h index 2bacc5bba4ae..70a37d18ba1a 100644 --- a/source/common/stats/stats_impl.h +++ b/source/common/stats/stats_impl.h @@ -191,7 +191,7 @@ class Utility { * it can be allocated from shared memory if needed. * * @note Due to name_ being variable size, sizeof(RawStatData) probably isn't useful. Use - * RawStatData::sizeGivenName() or RawStatData::sizeGivenStatsOptions() instead. + * RawStatData::structSize() or RawStatData::structSizeWithOptions() instead. */ struct RawStatData { @@ -207,14 +207,13 @@ struct RawStatData { * and padding for alignment. Required for the HeapRawStatDataAllocator, which does not truncate * at a maximum stat name length. */ - static uint64_t sizeGivenName(const absl::string_view name); + static uint64_t structSize(uint64_t name_size); /** - * Returns the size of this struct, accounting for the length of name_ - * and padding for alignment. Required by BlockMemoryHashSet, which has the context to report - * StatsOptions. + * Wrapper for structSize, taking a StatsOptions struct. + * Required by BlockMemoryHashSet, which has the context to supply StatsOptions. */ - static uint64_t sizeGivenStatsOptions(const StatsOptions& stats_options); + static uint64_t structSizeWithOptions(const StatsOptions& stats_options); /** * Initializes this object to have the specified key, diff --git a/source/server/hot_restart_impl.cc b/source/server/hot_restart_impl.cc index ef1926b6238d..e41db9566a6e 100644 --- a/source/server/hot_restart_impl.cc +++ b/source/server/hot_restart_impl.cc @@ -40,7 +40,7 @@ static BlockMemoryHashSetOptions blockMemHashOptions(uint64_t max_stats) { SharedMemory& SharedMemory::initialize(uint64_t stats_set_size, Options& options) { Api::OsSysCalls& os_sys_calls = Api::OsSysCallsSingleton::get(); - const uint64_t entry_size = Stats::RawStatData::sizeGivenStatsOptions(options.statsOptions()); + const uint64_t entry_size = Stats::RawStatData::structSizeWithOptions(options.statsOptions()); const uint64_t total_size = sizeof(SharedMemory) + stats_set_size; int flags = O_RDWR; @@ -172,7 +172,7 @@ void HotRestartImpl::free(Stats::RawStatData& data) { bool key_removed = stats_set_->remove(data.key()); ASSERT(key_removed); memset(static_cast(&data), 0, - Stats::RawStatData::sizeGivenStatsOptions(options_.statsOptions())); + Stats::RawStatData::structSizeWithOptions(options_.statsOptions())); } int HotRestartImpl::bindDomainSocket(uint64_t id) { diff --git a/test/common/common/block_memory_hash_set_test.cc b/test/common/common/block_memory_hash_set_test.cc index 6af29b211044..da5477cf8325 100644 --- a/test/common/common/block_memory_hash_set_test.cc +++ b/test/common/common/block_memory_hash_set_test.cc @@ -27,7 +27,7 @@ class BlockMemoryHashSetTest : public testing::Test { memcpy(name, key.data(), xfer); name[xfer] = '\0'; } - static uint64_t sizeGivenStatsOptions(const Stats::StatsOptions& stats_options) { + static uint64_t structSizeWithOptions(const Stats::StatsOptions& stats_options) { UNREFERENCED_PARAMETER(stats_options); return sizeof(TestValue); } diff --git a/test/common/stats/stats_impl_test.cc b/test/common/stats/stats_impl_test.cc index 44f82c5a6e43..eb25227e5d58 100644 --- a/test/common/stats/stats_impl_test.cc +++ b/test/common/stats/stats_impl_test.cc @@ -528,7 +528,7 @@ TEST(RawStatDataTest, Truncate) { Stats::StatsOptionsImpl stats_options; const std::string long_string(stats_options.maxNameLength() + 1, 'A'); RawStatData* stat = - static_cast(::calloc(RawStatData::sizeGivenStatsOptions(stats_options), 1)); + static_cast(::calloc(RawStatData::structSizeWithOptions(stats_options), 1)); EXPECT_LOG_CONTAINS("warning", "is too long with", stat->truncateAndInit(long_string, stats_options)); ::free(stat); diff --git a/test/test_common/utility.h b/test/test_common/utility.h index 5d7c3bbd477b..c972c9ad407c 100644 --- a/test/test_common/utility.h +++ b/test/test_common/utility.h @@ -347,7 +347,7 @@ class TestAllocator : public RawStatDataAllocator { CSmartPtr& stat_ref = stats_[name]; if (!stat_ref) { stat_ref.reset(static_cast( - ::calloc(RawStatData::sizeGivenStatsOptions(stats_options), 1))); + ::calloc(RawStatData::structSizeWithOptions(stats_options), 1))); stat_ref->truncateAndInit(name, stats_options); } else { stat_ref->ref_count_++;