From 8daaf6a2d3adfb0f4cb0b1e496a34c17ef680354 Mon Sep 17 00:00:00 2001 From: Binbin Date: Thu, 20 Apr 2023 15:29:33 +0800 Subject: [PATCH 1/3] Fix CONFIG REWRITE append duplicate newlines In the old code, everytime we perform a config rewrite, an additional newline is appended, see #1396 Because iostream::eof will only return true after reading the end of the stream. It does not indicate, that the next read will be the end of the stream. So everytime config rewrite is performed, the old loop will append a newline in `lines`, and then we will append it to the conf file. In addition, the same modification has been made to other similar places. This PR fixes #1396 --- src/cluster/cluster.cc | 4 ++-- src/config/config.cc | 4 ++-- utils/kvrocks2redis/config.cc | 4 ++-- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc index b98a6ca607e..89af7f7c247 100644 --- a/src/cluster/cluster.cc +++ b/src/cluster/cluster.cc @@ -613,8 +613,8 @@ Status Cluster::LoadClusterNodes(const std::string &file_path) { int64_t version = -1; std::string id, nodes_info; std::string line; - while (!file.eof()) { - std::getline(file, line); + while (std::getline(file, line)) { + if (file.eof()) break; auto parsed = ParseConfigLine(line); if (!parsed) return parsed.ToStatus().Prefixed("malformed line"); diff --git a/src/config/config.cc b/src/config/config.cc index 3e3c2866f91..77825aea073 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -829,8 +829,8 @@ Status Config::Rewrite() { std::ifstream file(path_); if (file.is_open()) { std::string raw_line; - while (!file.eof()) { - std::getline(file, raw_line); + while (std::getline(file, raw_line)) { + if (file.eof()) break; auto parsed = ParseConfigLine(raw_line); if (!parsed || parsed->first.empty()) { lines.emplace_back(raw_line); diff --git a/utils/kvrocks2redis/config.cc b/utils/kvrocks2redis/config.cc index efa75116c2c..6a8974ff01b 100644 --- a/utils/kvrocks2redis/config.cc +++ b/utils/kvrocks2redis/config.cc @@ -126,8 +126,8 @@ Status Config::Load(std::string path) { std::string line; int line_num = 1; - while (!file.eof()) { - std::getline(file, line); + while (std::getline(file, line)) { + if (file.eof()) break; Status s = parseConfigFromString(line); if (!s.IsOK()) { return s.Prefixed(fmt::format("at line #L{}", line_num)); From bf58fa94146ca00bd1fb61a06939f856987229ee Mon Sep 17 00:00:00 2001 From: Binbin Date: Thu, 20 Apr 2023 18:03:47 +0800 Subject: [PATCH 2/3] changing it to use file.good, thanks PragmaTwice --- src/cluster/cluster.cc | 4 +--- src/config/config.cc | 6 ++---- utils/kvrocks2redis/config.cc | 3 +-- 3 files changed, 4 insertions(+), 9 deletions(-) diff --git a/src/cluster/cluster.cc b/src/cluster/cluster.cc index 89af7f7c247..724a6045fe9 100644 --- a/src/cluster/cluster.cc +++ b/src/cluster/cluster.cc @@ -613,9 +613,7 @@ Status Cluster::LoadClusterNodes(const std::string &file_path) { int64_t version = -1; std::string id, nodes_info; std::string line; - while (std::getline(file, line)) { - if (file.eof()) break; - + while (file.good() && std::getline(file, line)) { auto parsed = ParseConfigLine(line); if (!parsed) return parsed.ToStatus().Prefixed("malformed line"); if (parsed->first.empty() || parsed->second.empty()) continue; diff --git a/src/config/config.cc b/src/config/config.cc index 77825aea073..f9768e853c1 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -726,8 +726,7 @@ Status Config::Load(const CLIOptions &opts) { std::string line; int line_num = 1; - while (!in->eof()) { - std::getline(*in, line); + while (in.good() && std::getline(*in, line)) { if (auto s = parseConfigFromString(line, line_num); !s.IsOK()) { return s.Prefixed(fmt::format("at line #L{}", line_num)); } @@ -829,8 +828,7 @@ Status Config::Rewrite() { std::ifstream file(path_); if (file.is_open()) { std::string raw_line; - while (std::getline(file, raw_line)) { - if (file.eof()) break; + while (file.good() && std::getline(file, raw_line)) { auto parsed = ParseConfigLine(raw_line); if (!parsed || parsed->first.empty()) { lines.emplace_back(raw_line); diff --git a/utils/kvrocks2redis/config.cc b/utils/kvrocks2redis/config.cc index 6a8974ff01b..907c9495ee1 100644 --- a/utils/kvrocks2redis/config.cc +++ b/utils/kvrocks2redis/config.cc @@ -126,8 +126,7 @@ Status Config::Load(std::string path) { std::string line; int line_num = 1; - while (std::getline(file, line)) { - if (file.eof()) break; + while (file.good() && std::getline(file, line)) { Status s = parseConfigFromString(line); if (!s.IsOK()) { return s.Prefixed(fmt::format("at line #L{}", line_num)); From 18b71a9c84fa194c964425ce4105ea07305ebe43 Mon Sep 17 00:00:00 2001 From: hulk Date: Thu, 20 Apr 2023 19:03:31 +0800 Subject: [PATCH 3/3] Update src/config/config.cc --- src/config/config.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/config/config.cc b/src/config/config.cc index f9768e853c1..8016b06330c 100644 --- a/src/config/config.cc +++ b/src/config/config.cc @@ -726,7 +726,7 @@ Status Config::Load(const CLIOptions &opts) { std::string line; int line_num = 1; - while (in.good() && std::getline(*in, line)) { + while (in->good() && std::getline(*in, line)) { if (auto s = parseConfigFromString(line, line_num); !s.IsOK()) { return s.Prefixed(fmt::format("at line #L{}", line_num)); }