You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@kvrocks.apache.org by GitBox <gi...@apache.org> on 2022/11/14 10:13:09 UTC

[GitHub] [incubator-kvrocks] torwig commented on a diff in pull request #1121: Fix `modernize-avoid-bind` warning reported by clang-tidy

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


##########
src/cluster/cluster.cc:
##########
@@ -319,7 +319,8 @@ Status Cluster::ImportSlot(Redis::Connection *conn, int slot, int state) {
       conn->SetImporting();
       myself_->importing_slot_ = slot;
       // Set link error callback
-      conn->close_cb_ = std::bind(&SlotImport::StopForLinkError, svr_->slot_import_, conn->GetFD());
+      // conn->close_cb_ = std::bind(&SlotImport::StopForLinkError, svr_->slot_import_, conn->GetFD());
+      conn->close_cb_ = [ObjectPtr = svr_->slot_import_, captureFd = conn->GetFD()](int fd) {ObjectPtr->StopForLinkError(captureFd);};

Review Comment:
   To be consistent, please use variables `object_ptr` (snake case) instead of `ObjectPtr` (camel case). `captureFd` as well.



##########
src/commands/redis_cmd.cc:
##########
@@ -3730,7 +3730,10 @@ class CommandUnSubscribe : public Commander {
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     if (args_.size() == 1) {
       conn->UnSubscribeAll(
-          std::bind(SubscribeCommandReply, output, "unsubscribe", std::placeholders::_1, std::placeholders::_2));
+          [output](const std::string &sub_name, int num){SubscribeCommandReply(output, "unsubscribe", sub_name, num);}
+      );
+      // conn->UnSubscribeAll(

Review Comment:
   I think that it's better to remove the code than comment it out.



##########
src/commands/redis_cmd.cc:
##########
@@ -3758,7 +3761,10 @@ class CommandPUnSubscribe : public Commander {
   Status Execute(Server *svr, Connection *conn, std::string *output) override {
     if (args_.size() == 1) {
       conn->PUnSubscribeAll(
-          std::bind(SubscribeCommandReply, output, "punsubscribe", std::placeholders::_1, std::placeholders::_2));
+          [output](const std::string &sub_name, int num) {SubscribeCommandReply(output, "punsubscribe", sub_name, num);}
+      );
+      // conn->PUnSubscribeAll(

Review Comment:
   And here as well (remove instead of comment out).



-- 
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