[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1192: Make IO functions return StatusOr

[GitBox <gi...@apache.org>]

torwig commented on code in PR #1192:
URL: https://github.com/apache/incubator-kvrocks/pull/1192#discussion_r1051426204


##########
src/common/io_util.cc:
##########
@@ -153,8 +153,7 @@ Status SockConnect(const std::string &host, uint32_t port, int *fd, int conn_tim
         return Status(Status::NotOK, std::string("setsockopt failed: ") + strerror(errno));

Review Comment:
   Could you use `Status::FromErrno` here as well?



##########
utils/kvrocks2redis/sync.cc:
##########
@@ -105,14 +106,10 @@ Status Sync::auth() {
   if (!config_->kvrocks_auth.empty()) {
     const auto auth_command = Redis::MultiBulkString({"AUTH", config_->kvrocks_auth});
     auto s = Util::SockSend(sock_fd_, auth_command);
-    if (!s.IsOK()) return Status(Status::NotOK, "send auth command err:" + s.Msg());
-    std::string line;
-    s = Util::SockReadLine(sock_fd_, &line);
-    if (!s.IsOK()) {
-      return Status(Status::NotOK, std::string("read auth response err: ") + s.Msg());
-    }
+    if (!s.IsOK()) s.Prefixed("send auth command err");

Review Comment:
   Perhaps you forgot the `return`  keyword before `s.Prefixed(...)`



-- 
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: issues-unsubscribe@kvrocks.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org