empiredan commented on code in PR #1811:
URL: 
https://github.com/apache/incubator-pegasus/pull/1811#discussion_r1436417805


##########
src/runtime/rpc/rpc_host_port.cpp:
##########
@@ -86,6 +87,33 @@ host_port::host_port(rpc_address addr)
     _type = addr.type();
 }
 
+bool host_port::from_string(const std::string s)
+{
+    auto pos = s.find_last_of(':');
+    if (pos == std::string::npos) {
+        return false;
+    }
+    _host = s.substr(0, pos);
+    std::string port = s.substr(pos + 1);
+
+    if (!internal::buf2unsigned(port, _port) || _port > UINT16_MAX) {

Review Comment:
   Consider adding a new function `buf2uint16` ? In principle, 
`internal::buf2unsigned` is an internal function and should be used only in 
`src/utils/string_conv.h`.
   
   On the other hand, there is no need to check the validity of `_port`, which 
has been done in `buf2unsigned`:
   ```suggestion
       if (!buf2uint16(port, _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: 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

Reply via email to