From ab9520df9bb47be2eb7d5720098ff463cd83ec36 Mon Sep 17 00:00:00 2001 From: Wu Tao Date: Mon, 11 Jan 2021 01:41:48 -0600 Subject: [PATCH] fix: multiple asan errors (#715) --- include/dsn/utility/TokenBucket.h | 6 +++++- include/dsn/utils/time_utils.h | 3 +++ src/aio/test/aio.cpp | 7 ++++--- src/runtime/rpc/rpc_message.cpp | 9 --------- src/utils/logging.cpp | 13 +------------ src/utils/simple_logger.cpp | 19 ++++++++++--------- src/utils/test/priority_queue.cpp | 1 + src/utils/time_utils.cpp | 11 +++++++++++ 8 files changed, 35 insertions(+), 34 deletions(-) diff --git a/include/dsn/utility/TokenBucket.h b/include/dsn/utility/TokenBucket.h index 5dc4e82efe..aec3fae99f 100644 --- a/include/dsn/utility/TokenBucket.h +++ b/include/dsn/utility/TokenBucket.h @@ -234,7 +234,11 @@ class BasicDynamicTokenBucket assert(burstSize > 0); if (burstSize < toConsume) { - return boost::none; + // boost::none + // if we use boost::none here, some compilers will generate warning + // that's actually a false positive of "-Wmaybe-uninitialized". + // https://www.boost.org/doc/libs/1_65_1/libs/optional/doc/html/boost_optional/tutorial/gotchas/false_positive_with__wmaybe_uninitialized.html + return boost::make_optional(false, double()); } while (toConsume > 0) { diff --git a/include/dsn/utils/time_utils.h b/include/dsn/utils/time_utils.h index 14732d2057..41e93083db 100644 --- a/include/dsn/utils/time_utils.h +++ b/include/dsn/utils/time_utils.h @@ -39,7 +39,10 @@ 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. 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); // get date string with format of 'yyyy-MM-dd' from given timestamp inline void time_ms_to_date(uint64_t ts_ms, char *str, int len) diff --git a/src/aio/test/aio.cpp b/src/aio/test/aio.cpp index 52bb3d70ca..4630e20008 100644 --- a/src/aio/test/aio.cpp +++ b/src/aio/test/aio.cpp @@ -26,6 +26,7 @@ #include #include +#include #include @@ -145,9 +146,9 @@ TEST(core, operation_failed) auto fp = file::open("tmp_test_file", O_WRONLY, 0600); EXPECT_TRUE(fp == nullptr); - ::dsn::error_code *err = new ::dsn::error_code; - size_t *count = new size_t; - auto io_callback = [err, count](::dsn::error_code e, size_t n) { + auto err = dsn::make_unique(); + auto count = dsn::make_unique(); + auto io_callback = [&err, &count](::dsn::error_code e, size_t n) { *err = e; *count = n; }; diff --git a/src/runtime/rpc/rpc_message.cpp b/src/runtime/rpc/rpc_message.cpp index db19b21da0..8964538060 100644 --- a/src/runtime/rpc/rpc_message.cpp +++ b/src/runtime/rpc/rpc_message.cpp @@ -24,15 +24,6 @@ * THE SOFTWARE. */ -/* - * Description: - * What is this file about? - * - * Revision history: - * xxxx-xx-xx, author, first version - * xxxx-xx-xx, author, fix bug about xxx - */ - #include #include #include diff --git a/src/utils/logging.cpp b/src/utils/logging.cpp index 8b0fbd3f21..84e33e4c77 100644 --- a/src/utils/logging.cpp +++ b/src/utils/logging.cpp @@ -45,18 +45,7 @@ using namespace tools; DSN_REGISTER_COMPONENT_PROVIDER(screen_logger, "dsn::tools::screen_logger"); DSN_REGISTER_COMPONENT_PROVIDER(simple_logger, "dsn::tools::simple_logger"); -std::function log_prefixed_message_func = []() { - static thread_local std::string prefixed_message; - - static thread_local std::once_flag flag; - std::call_once(flag, [&]() { - prefixed_message.resize(23); - int tid = dsn::utils::get_current_tid(); - sprintf(const_cast(prefixed_message.c_str()), "unknown.io-thrd.%05d: ", tid); - }); - - return prefixed_message; -}; +std::function log_prefixed_message_func = []() -> std::string { return ": "; }; void set_log_prefixed_message_func(std::function func) { diff --git a/src/utils/simple_logger.cpp b/src/utils/simple_logger.cpp index 9930bccfe0..9074b2ec2a 100644 --- a/src/utils/simple_logger.cpp +++ b/src/utils/simple_logger.cpp @@ -29,6 +29,7 @@ #include #include #include +#include namespace dsn { namespace tools { @@ -58,17 +59,17 @@ static void print_header(FILE *fp, dsn_log_level_t log_level) static char s_level_char[] = "IDWEF"; uint64_t ts = dsn_now_ns(); - char str[24]; - dsn::utils::time_ms_to_string(ts / 1000000, str); + std::string time_str; + dsn::utils::time_ms_to_string(ts / 1000000, time_str); int tid = dsn::utils::get_current_tid(); - fprintf(fp, - "%c%s (%" PRIu64 " %04x) %s", - s_level_char[log_level], - str, - ts, - tid, - log_prefixed_message_func().c_str()); + fmt::print(fp, + "{}{} ({} {}) {}", + s_level_char[log_level], + time_str, + ts, + tid, + log_prefixed_message_func()); } screen_logger::screen_logger(bool short_header) : logging_provider("./") diff --git a/src/utils/test/priority_queue.cpp b/src/utils/test/priority_queue.cpp index 1655bdeeaa..348abc2f11 100644 --- a/src/utils/test/priority_queue.cpp +++ b/src/utils/test/priority_queue.cpp @@ -114,6 +114,7 @@ TEST(core, blocking_priority_queue) ASSERT_EQ(0, ct); ASSERT_EQ(0, d->priority); ASSERT_EQ(10, d->queue_index); + delete d; bool flag = false; diff --git a/src/utils/time_utils.cpp b/src/utils/time_utils.cpp index 35a3650190..ed12fdef68 100644 --- a/src/utils/time_utils.cpp +++ b/src/utils/time_utils.cpp @@ -34,5 +34,16 @@ namespace utils { fmt::format_to(str, "{:%Y-%m-%d %H:%M:%S}.{}", *ret, static_cast(ts_ms % 1000)); } +/*extern*/ void time_ms_to_string(uint64_t ts_ms, std::string &str) +{ + str.clear(); + struct tm tmp; + auto ret = get_localtime(ts_ms, &tmp); + fmt::format_to(std::back_inserter(str), + "{:%Y-%m-%d %H:%M:%S}.{}", + *ret, + static_cast(ts_ms % 1000)); +} + } // namespace utils } // namespace dsn