empiredan commented on code in PR #1825:
URL:
https://github.com/apache/incubator-pegasus/pull/1825#discussion_r1441493401
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -114,6 +118,15 @@ TEST(host_port_test, rpc_group_host_port)
rpc_group_host_port *g = hp_grp.group_host_port();
ASSERT_EQ(std::string("test_group"), g->name());
+ host_port hp_grp1;
+ hp_grp1.assign_group("test_group");
+ if (reinterpret_cast<uint64_t>(hp_grp.group_host_port()) <
+ reinterpret_cast<uint64_t>(hp_grp1.group_host_port())) {
+ ASSERT_TRUE(hp_grp < hp_grp1);
Review Comment:
```suggestion
ASSERT_LT(hp_grp, hp_grp1);
```
##########
src/runtime/rpc/rpc_host_port.cpp:
##########
@@ -80,6 +81,7 @@ host_port::host_port(rpc_address addr)
} break;
case HOST_TYPE_GROUP: {
_group_host_port = new rpc_group_host_port(addr.group_address());
Review Comment:
Are there some special reasons why a raw pointer should be used here ?
Consider using the object instead ?
If a pointer is needed, consider `std::unique_ptr`, or use `dsn::ref_ptr` to
manage a `ref_counter` object at least ?
##########
src/runtime/test/host_port_test.cpp:
##########
@@ -114,6 +118,15 @@ TEST(host_port_test, rpc_group_host_port)
rpc_group_host_port *g = hp_grp.group_host_port();
ASSERT_EQ(std::string("test_group"), g->name());
+ host_port hp_grp1;
+ hp_grp1.assign_group("test_group");
+ if (reinterpret_cast<uint64_t>(hp_grp.group_host_port()) <
+ reinterpret_cast<uint64_t>(hp_grp1.group_host_port())) {
+ ASSERT_TRUE(hp_grp < hp_grp1);
+ } else {
+ ASSERT_FALSE(hp_grp < hp_grp1);
Review Comment:
```suggestion
ASSERT_GE(hp_grp, hp_grp1);
```
##########
src/runtime/rpc/rpc_host_port.h:
##########
@@ -118,6 +120,22 @@ inline bool operator==(const host_port &hp1, const
host_port &hp2)
inline bool operator!=(const host_port &hp1, const host_port &hp2) { return
!(hp1 == hp2); }
+inline bool operator<(const host_port &hp1, const host_port &hp2)
Review Comment:
Where would `operator<` be used ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: dev-unsubscr...@pegasus.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscr...@pegasus.apache.org
For additional commands, e-mail: dev-h...@pegasus.apache.org
