This is an automated email from the ASF dual-hosted git repository.
jamesge pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/brpc.git
The following commit(s) were added to refs/heads/master by this push:
new 1be060d5 reject initializing FlatMap when nbucket is 0
1be060d5 is described below
commit 1be060d57e2693b437fb4a7a774840d23e685fe4
Author: gejun.0 <[email protected]>
AuthorDate: Sat Mar 18 13:25:34 2023 +0800
reject initializing FlatMap when nbucket is 0
---
src/butil/containers/flat_map_inl.h | 8 ++++-
test/brpc_channel_unittest.cpp | 68 ++++++++++++++++++-------------------
test/flat_map_unittest.cpp | 55 +++++++++++++++++++++++-------
3 files changed, 84 insertions(+), 47 deletions(-)
diff --git a/src/butil/containers/flat_map_inl.h
b/src/butil/containers/flat_map_inl.h
index 3bd64e64..f79fcb86 100644
--- a/src/butil/containers/flat_map_inl.h
+++ b/src/butil/containers/flat_map_inl.h
@@ -41,6 +41,7 @@ inline uint32_t find_next_prime(uint32_t nbucket) {
return nbucket;
}
+// NOTE: find_power2(0) = 0
inline uint64_t find_power2(uint64_t b) {
b -= 1;
b |= (b >> 1);
@@ -61,7 +62,8 @@ inline size_t flatmap_round(size_t nbucket) {
#ifdef FLAT_MAP_ROUND_BUCKET_BY_USE_NEXT_PRIME
return find_next_prime(nbucket);
#else
- return find_power2(nbucket);
+ // the lowerbound fixes the corner case of nbucket=0 which results in
coredump during seeking the map.
+ return nbucket <= 8 ? 8 : find_power2(nbucket);
#endif
}
@@ -328,6 +330,10 @@ int FlatMap<_K, _T, _H, _E, _S, _A>::init(size_t nbucket,
u_int load_factor) {
LOG(ERROR) << "Already initialized";
return -1;
}
+ if (nbucket == 0) {
+ LOG(WARNING) << "Fail to init FlatMap, nbucket=" << nbucket;
+ return -1;
+ }
if (load_factor < 10 || load_factor > 100) {
LOG(ERROR) << "Invalid load_factor=" << load_factor;
return -1;
diff --git a/test/brpc_channel_unittest.cpp b/test/brpc_channel_unittest.cpp
index 644e983b..4de8e350 100644
--- a/test/brpc_channel_unittest.cpp
+++ b/test/brpc_channel_unittest.cpp
@@ -1981,46 +1981,46 @@ TEST_F(ChannelTest, parse_hostname) {
ASSERT_EQ(0, channel.Init("localhost:8888", &opt));
ASSERT_EQ("localhost:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com";, &opt));
- ASSERT_EQ("baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com:80";, &opt));
- ASSERT_EQ("baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com";, 80, &opt));
- ASSERT_EQ("baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com:8888";, &opt));
- ASSERT_EQ("baidu.com:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com";, 8888, &opt));
- ASSERT_EQ("baidu.com:8888", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com";, "rr", &opt));
- ASSERT_EQ("baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com:80";, "rr", &opt));
- ASSERT_EQ("baidu.com:80", channel._service_name);
- ASSERT_EQ(0, channel.Init("http://baidu.com:8888";, "rr", &opt));
- ASSERT_EQ("baidu.com:8888", channel._service_name);
-
- ASSERT_EQ(0, channel.Init("https://baidu.com";, &opt));
- ASSERT_EQ("baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com:443";, &opt));
- ASSERT_EQ("baidu.com:443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com";, 443, &opt));
- ASSERT_EQ("baidu.com:443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com:1443";, &opt));
- ASSERT_EQ("baidu.com:1443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com";, 1443, &opt));
- ASSERT_EQ("baidu.com:1443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com";, "rr", &opt));
- ASSERT_EQ("baidu.com", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com:443";, "rr", &opt));
- ASSERT_EQ("baidu.com:443", channel._service_name);
- ASSERT_EQ(0, channel.Init("https://baidu.com:1443";, "rr", &opt));
- ASSERT_EQ("baidu.com:1443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 80, &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, 8888, &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:80";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:80", channel._service_name);
+ ASSERT_EQ(0, channel.Init("http://www.baidu.com:8888";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:8888", channel._service_name);
+
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 443, &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, 1443, &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:443";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:443", channel._service_name);
+ ASSERT_EQ(0, channel.Init("https://www.baidu.com:1443";, "rr", &opt));
+ ASSERT_EQ("www.baidu.com:1443", channel._service_name);
const char *address_list[] = {
"10.127.0.1:1234",
"10.128.0.1:1234 enable",
"10.129.0.1:1234",
"localhost:1234",
- "baidu.com:1234"
+ "www.baidu.com:1234"
};
butil::TempFile tmp_file;
{
diff --git a/test/flat_map_unittest.cpp b/test/flat_map_unittest.cpp
index ac1b9bb6..6ba7b3ec 100644
--- a/test/flat_map_unittest.cpp
+++ b/test/flat_map_unittest.cpp
@@ -122,45 +122,71 @@ TEST_F(FlatMapTest, copy_flat_map) {
Map m1;
ASSERT_EQ(0, m1.init(16));
- m1["hello"] = "world";
- m1["foo"] = "bar";
+ size_t expected_count = 0;
+ m1["hello"] = "world"; ++expected_count;
+ m1["foo"] = "bar"; ++ expected_count;
+ m1["friend"] = "alice"; ++ expected_count;
+ m1["zone"] = "bj-01"; ++ expected_count;
+ m1["city"] = "shanghai"; ++ expected_count;
+ m1["owner"] = "bob"; ++ expected_count;
+ m1["lang"] = "chinese"; ++ expected_count;
ASSERT_TRUE(m1.initialized());
- ASSERT_EQ(2u, m1.size());
+ ASSERT_EQ(expected_count, m1.size());
// self assignment does nothing.
m1 = m1;
- ASSERT_EQ(2u, m1.size());
+ ASSERT_EQ(expected_count, m1.size());
ASSERT_EQ("world", m1["hello"]);
ASSERT_EQ("bar", m1["foo"]);
+ ASSERT_EQ("bob", m1["owner"]);
+ ASSERT_EQ("bj-01", m1["zone"]);
+ ASSERT_EQ("shanghai", m1["city"]);
+ ASSERT_EQ("chinese", m1["lang"]);
+ ASSERT_EQ("alice", m1["friend"]);
// Copy construct from initialized map.
Map m2 = m1;
ASSERT_TRUE(m2.initialized());
- ASSERT_EQ(2u, m2.size());
+ ASSERT_EQ(expected_count, m2.size());
ASSERT_EQ("world", m2["hello"]);
ASSERT_EQ("bar", m2["foo"]);
+ ASSERT_EQ("bob", m2["owner"]);
+ ASSERT_EQ("bj-01", m2["zone"]);
+ ASSERT_EQ("shanghai", m2["city"]);
+ ASSERT_EQ("chinese", m2["lang"]);
+ ASSERT_EQ("alice", m2["friend"]);
// assign initialized map to uninitialized map.
Map m3;
m3 = m1;
ASSERT_TRUE(m3.initialized());
- ASSERT_EQ(2u, m3.size());
+ ASSERT_EQ(expected_count, m3.size());
ASSERT_EQ("world", m3["hello"]);
ASSERT_EQ("bar", m3["foo"]);
+ ASSERT_EQ("bob", m3["owner"]);
+ ASSERT_EQ("bj-01", m3["zone"]);
+ ASSERT_EQ("shanghai", m3["city"]);
+ ASSERT_EQ("chinese", m3["lang"]);
+ ASSERT_EQ("alice", m3["friend"]);
// assign initialized map to initialized map (triggering resize)
Map m4;
ASSERT_EQ(0, m4.init(2));
- ASSERT_LT(m4.bucket_count(), m1.bucket_count());
+ ASSERT_LE(m4.bucket_count(), m1.bucket_count());
const void* old_buckets4 = m4._buckets;
m4 = m1;
ASSERT_EQ(m1.bucket_count(), m4.bucket_count());
ASSERT_NE(old_buckets4, m4._buckets);
- ASSERT_EQ(2u, m4.size());
+ ASSERT_EQ(expected_count, m4.size());
ASSERT_EQ("world", m4["hello"]);
ASSERT_EQ("bar", m4["foo"]);
+ ASSERT_EQ("bob", m4["owner"]);
+ ASSERT_EQ("bj-01", m4["zone"]);
+ ASSERT_EQ("shanghai", m4["city"]);
+ ASSERT_EQ("chinese", m4["lang"]);
+ ASSERT_EQ("alice", m4["friend"]);
// assign initialized map to initialized map (no resize)
- const size_t bcs[] = { 8, m1.bucket_count(), 32 };
+ const size_t bcs[] = { m1.bucket_count(), 32 };
// less than m1.bucket_count but enough for holding the elements
- ASSERT_LT(bcs[0], m1.bucket_count());
+ ASSERT_LE(bcs[0], m1.bucket_count());
// larger than m1.bucket_count
- ASSERT_GT(bcs[2], m1.bucket_count());
+ ASSERT_GE(bcs[1], m1.bucket_count());
for (size_t i = 0; i < arraysize(bcs); ++i) {
Map m5;
ASSERT_EQ(0, m5.init(bcs[i]));
@@ -169,9 +195,14 @@ TEST_F(FlatMapTest, copy_flat_map) {
m5 = m1;
ASSERT_EQ(old_bucket_count5, m5.bucket_count());
ASSERT_EQ(old_buckets5, m5._buckets);
- ASSERT_EQ(2u, m5.size());
+ ASSERT_EQ(expected_count, m5.size());
ASSERT_EQ("world", m5["hello"]);
ASSERT_EQ("bar", m5["foo"]);
+ ASSERT_EQ("bob", m5["owner"]);
+ ASSERT_EQ("bj-01", m5["zone"]);
+ ASSERT_EQ("shanghai", m5["city"]);
+ ASSERT_EQ("chinese", m5["lang"]);
+ ASSERT_EQ("alice", m5["friend"]);
}
}
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
