RiversJin commented on code in PR #2850:
URL: https://github.com/apache/kvrocks/pull/2850#discussion_r2009093589


##########
src/commands/cmd_replication.cc:
##########
@@ -166,41 +177,66 @@ class CommandReplConf : public Commander {
     return Commander::Parse(args);
   }
 
-  Status ParseParam(const std::string &option, const std::string &value) {
+  Status ParseParam(std::string_view option, std::string_view value) {
     if (option == "listening-port") {
       auto parse_result = ParseInt<int>(value, NumericRange<int>{1, PORT_LIMIT 
- 1}, 10);
       if (!parse_result) {
         return {Status::RedisParseErr, "listening-port should be number or out 
of range"};
       }
-
       port_ = *parse_result;
-    } else if (option == "ip-address") {
-      if (value == "") {
+      return Status::OK();
+    }
+
+    if (option == "ip-address") {
+      if (value.empty()) {
         return {Status::RedisParseErr, "ip-address should not be empty"};
       }
-      ip_address_ = value;
-    } else {
-      return {Status::RedisParseErr, errUnknownOption};
+      ip_ = value;
+      return Status::OK();
     }
 
-    return Status::OK();
+    if (option == "peer-id") {
+      if (value.empty()) {
+        return {Status::RedisParseErr, "peer-id should not be empty"};
+      }
+      peer_id_ = value;
+      return Status::OK();
+    }
+
+    if (option == "version") {
+      auto parse_result = ParseInt<int64_t>(value, 10);
+      if (!parse_result) {
+        return {Status::RedisParseErr, "version should be number"};
+      }
+      peer_version_ = *parse_result;
+      return Status::OK();
+    }
+
+    return {Status::RedisParseErr, errUnknownOption};
   }
 
   Status Execute([[maybe_unused]] engine::Context &ctx, [[maybe_unused]] 
Server *srv, Connection *conn,
                  std::string *output) override {
-    if (port_ != 0) {

Review Comment:
   Based on the `replconf` command definition, it's impossible to reach this 
code path without `port_` being set, so I've removed the validation check for 
`port_`.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to