This is an automated email from the ASF dual-hosted git repository. djwang pushed a commit to branch main in repository https://gitbox.apache.org/repos/asf/cloudberry.git
commit e585e6a6ca76fe0209427001909be1fb063e9ccc Author: NJrslv <[email protected]> AuthorDate: Mon Jun 16 13:07:59 2025 +0300 [yagp_hooks_collector] Miscellaneous fixes and refactoring Fix UB in strcpy. General code refactoring. --- src/Config.cpp | 44 +++++++++++++++++++++++++++++++++++++++++--- src/Config.h | 2 +- src/EventSender.cpp | 39 +-------------------------------------- src/EventSender.h | 1 - src/UDSConnector.cpp | 8 +++++++- 5 files changed, 50 insertions(+), 44 deletions(-) diff --git a/src/Config.cpp b/src/Config.cpp index 19aa37d1b9d..5e0749f171d 100644 --- a/src/Config.cpp +++ b/src/Config.cpp @@ -6,6 +6,7 @@ extern "C" { #include "postgres.h" +#include "utils/builtins.h" #include "utils/guc.h" } @@ -16,8 +17,39 @@ static bool guc_enable_collector = true; static bool guc_report_nested_queries = true; static char *guc_ignored_users = nullptr; static int guc_max_text_size = 1024; // in KB -std::unique_ptr<std::unordered_set<std::string>> ignored_users_set = nullptr; -bool ignored_users_guc_dirty = false; +static std::unique_ptr<std::unordered_set<std::string>> ignored_users_set = + nullptr; +static bool ignored_users_guc_dirty = false; + +static void update_ignored_users(const char *new_guc_ignored_users) { + auto new_ignored_users_set = + std::make_unique<std::unordered_set<std::string>>(); + if (new_guc_ignored_users != nullptr && new_guc_ignored_users[0] != '\0') { + /* Need a modifiable copy of string */ + char *rawstring = pstrdup(new_guc_ignored_users); + List *elemlist; + ListCell *l; + + /* Parse string into list of identifiers */ + if (!SplitIdentifierString(rawstring, ',', &elemlist)) { + /* syntax error in list */ + pfree(rawstring); + list_free(elemlist); + ereport( + LOG, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg( + "invalid list syntax in parameter yagpcc.ignored_users_list"))); + return; + } + foreach (l, elemlist) { + new_ignored_users_set->insert((char *)lfirst(l)); + } + pfree(rawstring); + list_free(elemlist); + } + ignored_users_set = std::move(new_ignored_users_set); +} static void assign_ignored_users_hook(const char *, void *) { ignored_users_guc_dirty = true; @@ -67,7 +99,6 @@ bool Config::enable_analyze() { return guc_enable_analyze; } bool Config::enable_cdbstats() { return guc_enable_cdbstats; } bool Config::enable_collector() { return guc_enable_collector; } bool Config::report_nested_queries() { return guc_report_nested_queries; } -const char *Config::ignored_users() { return guc_ignored_users; } size_t Config::max_text_size() { return guc_max_text_size * 1024; } bool Config::filter_user(const std::string *username) { @@ -76,3 +107,10 @@ bool Config::filter_user(const std::string *username) { } return ignored_users_set->find(*username) != ignored_users_set->end(); } + +void Config::sync() { + if (ignored_users_guc_dirty) { + update_ignored_users(guc_ignored_users); + ignored_users_guc_dirty = false; + } +} \ No newline at end of file diff --git a/src/Config.h b/src/Config.h index 9dd33c68321..3caa0c78339 100644 --- a/src/Config.h +++ b/src/Config.h @@ -11,6 +11,6 @@ public: static bool enable_collector(); static bool filter_user(const std::string *username); static bool report_nested_queries(); - static const char *ignored_users(); static size_t max_text_size(); + static void sync(); }; \ No newline at end of file diff --git a/src/EventSender.cpp b/src/EventSender.cpp index fed9b69911f..fc0f7e1aa07 100644 --- a/src/EventSender.cpp +++ b/src/EventSender.cpp @@ -8,7 +8,6 @@ extern "C" { #include "access/hash.h" #include "executor/executor.h" #include "utils/elog.h" -#include "utils/builtins.h" #include "cdb/cdbdisp.h" #include "cdb/cdbexplain.h" @@ -21,9 +20,6 @@ extern "C" { #include "PgUtils.h" #include "ProtoUtils.h" -extern std::unique_ptr<std::unordered_set<std::string>> ignored_users_set; -extern bool ignored_users_guc_dirty; - void EventSender::query_metrics_collect(QueryMetricsStatus status, void *arg) { if (Gp_role != GP_ROLE_DISPATCH && Gp_role != GP_ROLE_EXECUTE) { return; @@ -66,10 +62,7 @@ void EventSender::executor_before_start(QueryDesc *query_desc, nested_timing = 0; nested_calls = 0; } - if (ignored_users_guc_dirty) { - update_ignored_users(Config::ignored_users()); - ignored_users_guc_dirty = false; - } + Config::sync(); if (!need_collect(query_desc, nesting_level)) { return; } @@ -355,36 +348,6 @@ void EventSender::update_nested_counters(QueryDesc *query_desc) { } } -void EventSender::update_ignored_users(const char *new_guc_ignored_users) { - auto new_ignored_users_set = - std::make_unique<std::unordered_set<std::string>>(); - if (new_guc_ignored_users != nullptr && new_guc_ignored_users[0] != '\0') { - /* Need a modifiable copy of string */ - char *rawstring = pstrdup(new_guc_ignored_users); - List *elemlist; - ListCell *l; - - /* Parse string into list of identifiers */ - if (!SplitIdentifierString(rawstring, ',', &elemlist)) { - /* syntax error in list */ - pfree(rawstring); - list_free(elemlist); - ereport( - LOG, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg( - "invalid list syntax in parameter yagpcc.ignored_users_list"))); - return; - } - foreach (l, elemlist) { - new_ignored_users_set->insert((char *)lfirst(l)); - } - pfree(rawstring); - list_free(elemlist); - } - ignored_users_set = std::move(new_ignored_users_set); -} - EventSender::QueryItem::QueryItem(EventSender::QueryState st, yagpcc::SetQueryReq *msg) : state(st), message(msg) {} \ No newline at end of file diff --git a/src/EventSender.h b/src/EventSender.h index 6919defbbb3..99f7b24753d 100644 --- a/src/EventSender.h +++ b/src/EventSender.h @@ -57,7 +57,6 @@ private: void collect_query_done(QueryDesc *query_desc, QueryMetricsStatus status); void cleanup_messages(); void update_nested_counters(QueryDesc *query_desc); - void update_ignored_users(const char *new_guc_ignored_users); UDSConnector *connector = nullptr; int nesting_level = 0; diff --git a/src/UDSConnector.cpp b/src/UDSConnector.cpp index b9088205250..8a5f754f3b4 100644 --- a/src/UDSConnector.cpp +++ b/src/UDSConnector.cpp @@ -30,7 +30,13 @@ bool UDSConnector::report_query(const yagpcc::SetQueryReq &req, const std::string &event) { sockaddr_un address; address.sun_family = AF_UNIX; - strcpy(address.sun_path, Config::uds_path().c_str()); + std::string uds_path = Config::uds_path(); + if (uds_path.size() >= sizeof(address.sun_path)) { + ereport(WARNING, (errmsg("UDS path is too long for socket buffer"))); + YagpStat::report_error(); + return false; + } + strcpy(address.sun_path, uds_path.c_str()); bool success = true; auto sockfd = socket(AF_UNIX, SOCK_STREAM, 0); if (sockfd != -1) { --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
