[GitHub] [incubator-kvrocks] git-hulk commented on a diff in pull request #1112: Fix modernize-loop-convert warning reported by clang-tidy

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

git-hulk commented on code in PR #1112:
URL: https://github.com/apache/incubator-kvrocks/pull/1112#discussion_r1021223956


##########
src/types/redis_string.cc:
##########
@@ -137,8 +137,8 @@ std::vector<rocksdb::Status> String::MGet(const std::vector<Slice> &keys, std::v
   // don't use range-based for loop here, coz the slice member
   // would refer the address instead of copy the value, and use
   // range-based for loop may cause all members refer to the same addr
-  for (size_t i = 0; i < ns_keys.size(); i++) {
-    slice_keys.emplace_back(ns_keys[i]);
+  for (const auto &ns_key : ns_keys) {

Review Comment:
   @PragmaTwice @torwig @MaximSmolskiy If my memory serves, the root cause is the for loop would reuse the same variable `ns_key` address if we use `for (auto ns_key : ns_keys)` instead of `for (auto &ns_key : ns_keys)`.



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