You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@pegasus.apache.org by wa...@apache.org on 2023/05/06 06:18:26 UTC
[incubator-pegasus] branch master updated: fix: fix garble words of time filed in logs (#1468)
This is an automated email from the ASF dual-hosted git repository.
wangdan pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/incubator-pegasus.git
The following commit(s) were added to refs/heads/master by this push:
new 5d1ce0b42 fix: fix garble words of time filed in logs (#1468)
5d1ce0b42 is described below
commit 5d1ce0b42f4c1399c101e27b7e7db1f97e9e6c25
Author: Yingchun Lai <la...@apache.org>
AuthorDate: Sat May 6 14:18:21 2023 +0800
fix: fix garble words of time filed in logs (#1468)
https://github.com/apache/incubator-pegasus/issues/1467
Because `fmt::format_to()` does not append a terminating null character, it
may cause garble words if not initialize the buffer's memory as zero.
This patch initialize all callers of `time_ms_to_string`'s buffer memory as zero.
---
src/meta/meta_http_service.cpp | 6 +++---
src/meta/partition_guardian.cpp | 4 ++--
src/server/pegasus_manual_compact_service.cpp | 6 +++---
src/shell/commands/recovery.cpp | 2 +-
src/tools/mutation_log_tool.cpp | 2 +-
src/utils/process_utils.cpp | 4 ++--
src/utils/test/time_utils_test.cpp | 2 +-
src/utils/time_utils.cpp | 2 ++
src/utils/time_utils.h | 5 +++--
9 files changed, 18 insertions(+), 15 deletions(-)
diff --git a/src/meta/meta_http_service.cpp b/src/meta/meta_http_service.cpp
index 39eab4b70..902822d79 100644
--- a/src/meta/meta_http_service.cpp
+++ b/src/meta/meta_http_service.cpp
@@ -255,7 +255,7 @@ void meta_http_service::list_app_handler(const http_request &req, http_response
status_str = status_str.substr(status_str.find("AS_") + 3);
std::string create_time = "-";
if (app.create_second > 0) {
- char buf[24];
+ char buf[24] = {0};
dsn::utils::time_ms_to_string((uint64_t)app.create_second * 1000, buf);
create_time = buf;
}
@@ -265,12 +265,12 @@ void meta_http_service::list_app_handler(const http_request &req, http_response
available_app_count++;
} else if (app.status == app_status::AS_DROPPED && app.expire_second > 0) {
if (app.drop_second > 0) {
- char buf[24];
+ char buf[24] = {0};
dsn::utils::time_ms_to_string((uint64_t)app.drop_second * 1000, buf);
drop_time = buf;
}
if (app.expire_second > 0) {
- char buf[24];
+ char buf[24] = {0};
dsn::utils::time_ms_to_string((uint64_t)app.expire_second * 1000, buf);
drop_expire_time = buf;
}
diff --git a/src/meta/partition_guardian.cpp b/src/meta/partition_guardian.cpp
index e82ad8386..09b0862c4 100644
--- a/src/meta/partition_guardian.cpp
+++ b/src/meta/partition_guardian.cpp
@@ -310,7 +310,7 @@ pc_status partition_guardian::on_missing_primary(meta_view &view, const dsn::gpi
action.node.set_invalid();
for (int i = 0; i < cc.dropped.size(); ++i) {
const dropped_replica &dr = cc.dropped[i];
- char time_buf[30];
+ char time_buf[30] = {0};
::dsn::utils::time_ms_to_string(dr.time, time_buf);
LOG_INFO("{}: config_context.dropped[{}]: "
"node({}), time({})[{}], ballot({}), "
@@ -510,7 +510,7 @@ pc_status partition_guardian::on_missing_secondary(meta_view &view, const dsn::g
} else if (has_milliseconds_expired(cc.dropped.back().time +
_replica_assign_delay_ms_for_dropouts)) {
is_emergency = true;
- char time_buf[30];
+ char time_buf[30] = {0};
::dsn::utils::time_ms_to_string(cc.dropped.back().time, time_buf);
LOG_INFO("gpid({}): is emergency due to lose secondary for a long time, "
"last_dropped_node({}), drop_time({}), delay_ms({})",
diff --git a/src/server/pegasus_manual_compact_service.cpp b/src/server/pegasus_manual_compact_service.cpp
index 22a40afcb..643e3e997 100644
--- a/src/server/pegasus_manual_compact_service.cpp
+++ b/src/server/pegasus_manual_compact_service.cpp
@@ -336,7 +336,7 @@ std::string pegasus_manual_compact_service::query_compact_state() const
uint64_t last_time_used_ms = _manual_compact_last_time_used_ms.load();
std::stringstream state;
if (last_finish_time_ms > 0) {
- char str[24];
+ char str[24] = {0};
dsn::utils::time_ms_to_string(last_finish_time_ms, str);
state << "last finish at [" << str << "]";
} else {
@@ -348,13 +348,13 @@ std::string pegasus_manual_compact_service::query_compact_state() const
}
if (enqueue_time_ms > 0) {
- char str[24];
+ char str[24] = {0};
dsn::utils::time_ms_to_string(enqueue_time_ms, str);
state << ", recent enqueue at [" << str << "]";
}
if (start_time_ms > 0) {
- char str[24];
+ char str[24] = {0};
dsn::utils::time_ms_to_string(start_time_ms, str);
state << ", recent start at [" << str << "]";
}
diff --git a/src/shell/commands/recovery.cpp b/src/shell/commands/recovery.cpp
index 2100fa8c7..340b1ab7f 100644
--- a/src/shell/commands/recovery.cpp
+++ b/src/shell/commands/recovery.cpp
@@ -295,7 +295,7 @@ bool ddd_diagnose(command_executor *e, shell_context *sc, arguments args)
secondary_latest_dropped = pinfo.config.last_drops[pinfo.config.last_drops.size() - 2];
int j = 0;
for (const ddd_node_info &n : pinfo.dropped) {
- char time_buf[30];
+ char time_buf[30] = {0};
::dsn::utils::time_ms_to_string(n.drop_time_ms, time_buf);
out << " dropped[" << j++ << "]: "
<< "node(" << n.node.to_string() << "), "
diff --git a/src/tools/mutation_log_tool.cpp b/src/tools/mutation_log_tool.cpp
index 41aabd382..3c1177d4d 100644
--- a/src/tools/mutation_log_tool.cpp
+++ b/src/tools/mutation_log_tool.cpp
@@ -55,7 +55,7 @@ bool mutation_log_tool::dump(
if (mlog->max_decree(mu->data.header.pid) == 0) {
mlog->set_valid_start_offset_on_open(mu->data.header.pid, 0);
}
- char timestamp_buf[32];
+ char timestamp_buf[32] = {0};
utils::time_ms_to_string(mu->data.header.timestamp / 1000, timestamp_buf);
output << "mutation [" << mu->name() << "]: "
<< "gpid=" << mu->data.header.pid.get_app_id() << "."
diff --git a/src/utils/process_utils.cpp b/src/utils/process_utils.cpp
index 4ffbd9603..285956079 100644
--- a/src/utils/process_utils.cpp
+++ b/src/utils/process_utils.cpp
@@ -99,8 +99,8 @@ public:
mills = get_current_physical_time_ns() / 1000000;
time_ms_to_string(mills, date_time_mills);
}
- uint64_t mills;
- char date_time_mills[64];
+ uint64_t mills = 0;
+ char date_time_mills[64] = {0};
};
//
diff --git a/src/utils/test/time_utils_test.cpp b/src/utils/test/time_utils_test.cpp
index 3dac7c9b1..aea665243 100644
--- a/src/utils/test/time_utils_test.cpp
+++ b/src/utils/test/time_utils_test.cpp
@@ -91,7 +91,7 @@ TEST(time_utils, get_current_physical_time_ns)
TEST(time_utils, time_ms_to_string)
{
- char buf[64];
+ char buf[64] = {0};
time_ms_to_string(1605091506136, buf);
// time differ between time zones,
// the real time 2020-11-11 18:45:06.136 (UTC+8)
diff --git a/src/utils/time_utils.cpp b/src/utils/time_utils.cpp
index 091d9de46..711b4cd3d 100644
--- a/src/utils/time_utils.cpp
+++ b/src/utils/time_utils.cpp
@@ -37,6 +37,8 @@ namespace utils {
{
struct tm tmp;
auto ret = get_localtime(ts_ms, &tmp);
+ // NOTE: format_to() does not append a terminating null character, so remember to initialize
+ // str's memory as zero before.
fmt::format_to(str, "{:%Y-%m-%d %H:%M:%S}.{}", *ret, static_cast<uint32_t>(ts_ms % 1000));
}
diff --git a/src/utils/time_utils.h b/src/utils/time_utils.h
index 50df0b785..a47ba594c 100644
--- a/src/utils/time_utils.h
+++ b/src/utils/time_utils.h
@@ -44,8 +44,9 @@ static struct tm *get_localtime(uint64_t ts_ms, struct tm *tm_buf)
}
// get time string, which format is yyyy-MM-dd hh:mm:ss.SSS
-// NOTE: using char* as output is usually unsafe. Please use std::string as the output argument
-// as long as it's possible.
+// NOTE: using char* as output is usually unsafe, remember to initialize its memory as zero before
+// calling 'time_ms_to_string'. Please use std::string as the output argument as long as it's
+// possible.
extern void time_ms_to_string(uint64_t ts_ms, char *str);
extern void time_ms_to_string(uint64_t ts_ms, std::string &str);
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@pegasus.apache.org
For additional commands, e-mail: commits-help@pegasus.apache.org