From 727dc08bfa116d3c3d152b7e959df304ebbe0652 Mon Sep 17 00:00:00 2001 From: luopengting Date: Thu, 8 Apr 2021 10:56:32 +0800 Subject: [PATCH] fix TAINTED_SCALAR and capitalize constants 1. fix the tainted variable 'path' 2. capitalize constants in env_config_parser 3. move constants about attribute to common utils.h --- mindspore/ccsrc/debug/common.cc | 2 +- mindspore/ccsrc/debug/common.h | 6 +-- mindspore/ccsrc/debug/env_config_parser.cc | 42 +++++++++---------- mindspore/ccsrc/debug/rdr/base_recorder.cc | 4 +- mindspore/ccsrc/debug/rdr/base_recorder.h | 2 +- mindspore/ccsrc/debug/rdr/recorder_manager.h | 8 ++-- .../debug/rdr/stream_exec_order_recorder.h | 6 --- mindspore/ccsrc/utils/utils.h | 5 +++ 8 files changed, 37 insertions(+), 38 deletions(-) diff --git a/mindspore/ccsrc/debug/common.cc b/mindspore/ccsrc/debug/common.cc index a3141f8961..0c17d15506 100644 --- a/mindspore/ccsrc/debug/common.cc +++ b/mindspore/ccsrc/debug/common.cc @@ -239,7 +239,7 @@ bool Common::IsPathValid(const std::string &path, const int &length_limit, const return false; } - if (!IsEveryFilenameValid(path, maxOSFilenameLength, err_msg)) { + if (!IsEveryFilenameValid(path, MAX_OS_FILENAME_LENGTH, err_msg)) { return false; } return true; diff --git a/mindspore/ccsrc/debug/common.h b/mindspore/ccsrc/debug/common.h index a553254cf2..48d21b589c 100644 --- a/mindspore/ccsrc/debug/common.h +++ b/mindspore/ccsrc/debug/common.h @@ -22,9 +22,9 @@ #include "utils/contract.h" namespace mindspore { -static const int maxDirectoryLength = 1024; -static const int maxFilenameLength = 128; -static const int maxOSFilenameLength = 255; +static const int MAX_DIRECTORY_LENGTH = 1024; +static const int MAX_FILENAME_LENGTH = 128; +static const int MAX_OS_FILENAME_LENGTH = 255; class Common { public: Common() = default; diff --git a/mindspore/ccsrc/debug/env_config_parser.cc b/mindspore/ccsrc/debug/env_config_parser.cc index 50247dcc6a..8c90e997fe 100644 --- a/mindspore/ccsrc/debug/env_config_parser.cc +++ b/mindspore/ccsrc/debug/env_config_parser.cc @@ -23,22 +23,22 @@ #include "utils/convert_utils_base.h" namespace { -constexpr auto kEnableEnv = "MS_RDR_ENABLE"; -constexpr auto kPathEnv = "MS_RDR_PATH"; -constexpr auto kRdrSettings = "rdr"; -constexpr auto kPath = "path"; -constexpr auto kEnable = "enable"; +constexpr auto ENV_RDR_ENABLE = "MS_RDR_ENABLE"; +constexpr auto ENV_RDR_PATH = "MS_RDR_PATH"; +constexpr auto KEY_RDR_SETTINGS = "rdr"; +constexpr auto KEY_PATH = "path"; +constexpr auto KEY_ENABLE = "enable"; } // namespace namespace mindspore { std::optional GetRdrEnableFromEnv() { // get environment variable to configure RDR - const char *env_enable_char = std::getenv(kEnableEnv); + const char *env_enable_char = std::getenv(ENV_RDR_ENABLE); if (env_enable_char != nullptr) { - std::string env_enable_str = env_enable_char; + std::string env_enable_str = std::string(env_enable_char); (void)std::transform(env_enable_str.begin(), env_enable_str.end(), env_enable_str.begin(), ::tolower); if (env_enable_str != "0" && env_enable_str != "1") { - MS_LOG(WARNING) << "The environment variable '" << kEnableEnv << "' should be 0 or 1."; + MS_LOG(WARNING) << "The environment variable '" << ENV_RDR_ENABLE << "' should be 0 or 1."; } if (env_enable_str == "1") { return true; @@ -50,12 +50,12 @@ std::optional GetRdrEnableFromEnv() { std::optional GetRdrPathFromEnv() { // get environment variable to configure RDR - const char *path_char = std::getenv(kPathEnv); + const char *path_char = std::getenv(ENV_RDR_PATH); if (path_char != nullptr) { std::string err_msg = "RDR path parse from environment variable failed. Please check the settings about '" + - std::string(kPathEnv) + "' in environment variables."; - std::string path = path_char; - if (!Common::IsPathValid(path, maxDirectoryLength, err_msg)) { + std::string(ENV_RDR_PATH) + "' in environment variables."; + std::string path = std::string(path_char); + if (!Common::IsPathValid(path, MAX_DIRECTORY_LENGTH, err_msg)) { return std::string(""); } return path; @@ -155,21 +155,21 @@ void EnvConfigParser::Parse() { } void EnvConfigParser::ParseRdrSetting(const nlohmann::json &content) { - auto rdr_setting = content.find(kRdrSettings); + auto rdr_setting = content.find(KEY_RDR_SETTINGS); if (rdr_setting == content.end()) { - MS_LOG(WARNING) << "The '" << kRdrSettings << "' not exists. Please check the config file '" << config_file_ + MS_LOG(WARNING) << "The '" << KEY_RDR_SETTINGS << "' not exists. Please check the config file '" << config_file_ << "' set by 'env_config_path' in context."; return; } has_rdr_setting_ = true; - auto rdr_enable = CheckJsonKeyExist(*rdr_setting, kRdrSettings, kEnable); + auto rdr_enable = CheckJsonKeyExist(*rdr_setting, KEY_RDR_SETTINGS, KEY_ENABLE); if (rdr_enable.has_value()) { ParseRdrEnable(**rdr_enable); } - auto rdr_path = CheckJsonKeyExist(*rdr_setting, kRdrSettings, kPath); + auto rdr_path = CheckJsonKeyExist(*rdr_setting, KEY_RDR_SETTINGS, KEY_PATH); if (rdr_path.has_value()) { ParseRdrPath(**rdr_path); } @@ -177,16 +177,16 @@ void EnvConfigParser::ParseRdrSetting(const nlohmann::json &content) { void EnvConfigParser::ParseRdrPath(const nlohmann::json &content) { std::string err_msg = "RDR path parse failed. The RDR path will be a default value: '" + rdr_path_ + - "'. Please check the settings about '" + kRdrSettings + "' in config file '" + config_file_ + - "' set by 'env_config_path' in context."; + "'. Please check the settings about '" + KEY_RDR_SETTINGS + "' in config file '" + + config_file_ + "' set by 'env_config_path' in context."; - if (!CheckJsonStringType(content, kRdrSettings, kPath)) { + if (!CheckJsonStringType(content, KEY_RDR_SETTINGS, KEY_PATH)) { MS_LOG(WARNING) << err_msg; return; } std::string path = content; - if (!Common::IsPathValid(path, maxDirectoryLength, err_msg)) { + if (!Common::IsPathValid(path, MAX_DIRECTORY_LENGTH, err_msg)) { return; } @@ -198,7 +198,7 @@ void EnvConfigParser::ParseRdrPath(const nlohmann::json &content) { void EnvConfigParser::ParseRdrEnable(const nlohmann::json &content) { if (!content.is_boolean()) { - MS_LOG(WARNING) << "Json parse failed. 'enable' in " << kRdrSettings << " should be boolean." + MS_LOG(WARNING) << "Json parse failed. 'enable' in " << KEY_RDR_SETTINGS << " should be boolean." << " Please check the config file '" << config_file_ << "' set by 'env_config_path' in context."; return; } diff --git a/mindspore/ccsrc/debug/rdr/base_recorder.cc b/mindspore/ccsrc/debug/rdr/base_recorder.cc index 1314ca93f7..f90fb98fde 100644 --- a/mindspore/ccsrc/debug/rdr/base_recorder.cc +++ b/mindspore/ccsrc/debug/rdr/base_recorder.cc @@ -22,7 +22,7 @@ namespace mindspore { void BaseRecorder::SetDirectory(const std::string &directory) { std::string error_message = module_ + ":" + name_ + " set directory failed."; - if (Common::IsPathValid(directory, maxDirectoryLength, error_message)) { + if (Common::IsPathValid(directory, MAX_DIRECTORY_LENGTH, error_message)) { directory_ = directory; if (directory_.back() != '/') { directory_ += "/"; @@ -32,7 +32,7 @@ void BaseRecorder::SetDirectory(const std::string &directory) { void BaseRecorder::SetFilename(const std::string &filename) { std::string error_message = module_ + ":" + name_ + " set filename failed."; - if (Common::IsFilenameValid(filename, maxDirectoryLength, error_message)) { + if (Common::IsFilenameValid(filename, MAX_DIRECTORY_LENGTH, error_message)) { filename_ = filename; } } diff --git a/mindspore/ccsrc/debug/rdr/base_recorder.h b/mindspore/ccsrc/debug/rdr/base_recorder.h index 3a67f2a51a..805aa5d7de 100644 --- a/mindspore/ccsrc/debug/rdr/base_recorder.h +++ b/mindspore/ccsrc/debug/rdr/base_recorder.h @@ -39,7 +39,7 @@ class BaseRecorder { } std::string err_msg = module_ + ":" + name_ + " set filename failed."; - if (!filename_.empty() && !Common::IsFilenameValid(filename_, maxFilenameLength, err_msg)) { + if (!filename_.empty() && !Common::IsFilenameValid(filename_, MAX_FILENAME_LENGTH, err_msg)) { filename_ = ""; } diff --git a/mindspore/ccsrc/debug/rdr/recorder_manager.h b/mindspore/ccsrc/debug/rdr/recorder_manager.h index 363ceed96c..ed4c68f7ac 100644 --- a/mindspore/ccsrc/debug/rdr/recorder_manager.h +++ b/mindspore/ccsrc/debug/rdr/recorder_manager.h @@ -26,13 +26,13 @@ namespace mindspore { // The number is the reciprocal of the golden ratio. -const unsigned int magicConstant = 0x9e3779b9; -const unsigned int hashShiftLeft = 6; -const unsigned int hashShiftRight = 2; +const unsigned int MAGIC_CONSTANT = 0x9e3779b9; +const unsigned int HASH_SHIFT_LEFT = 6; +const unsigned int HASH_SHIFT_RIGHT = 2; template inline void hash_combine(std::size_t *seed, const T &val) { - *seed ^= std::hash()(val) + magicConstant + (*seed << hashShiftLeft) + (*seed >> hashShiftRight); + *seed ^= std::hash()(val) + MAGIC_CONSTANT + (*seed << HASH_SHIFT_LEFT) + (*seed >> HASH_SHIFT_RIGHT); } template diff --git a/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h b/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h index 7e692ac787..c67319a37f 100644 --- a/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h +++ b/mindspore/ccsrc/debug/rdr/stream_exec_order_recorder.h @@ -25,12 +25,6 @@ using json = nlohmann::json; -constexpr auto kAttrNodeName = "node_name"; -constexpr auto kAttrLogicId = "logic_id"; -constexpr auto kAttrNodeInfo = "node_info"; -constexpr auto kAttrLabelId = "label_id"; -constexpr auto kAttrActiveStreamId = "active_stream_id"; - namespace mindspore { class ExecNode { public: diff --git a/mindspore/ccsrc/utils/utils.h b/mindspore/ccsrc/utils/utils.h index baf4af9b61..8c5b6a79cd 100644 --- a/mindspore/ccsrc/utils/utils.h +++ b/mindspore/ccsrc/utils/utils.h @@ -325,10 +325,15 @@ constexpr auto kAttrAtomicWorkspaceIndexs = "atomic_workspace_clean_indexs"; constexpr auto kAttrSwitchCondition = "switch_condition"; constexpr auto kAttrDataType = "data_type"; constexpr auto kAttrActiveTarget = "active_target"; +constexpr auto kAttrActiveStreamId = "active_stream_id"; constexpr auto kAttrActiveStreamList = "active_stream_list"; constexpr auto kAttrTrueBranchStream = "true_branch_stream"; constexpr auto kAttrStreamSwitchKind = "stream_switch_kind"; constexpr auto kAttrEventId = "event_id"; +constexpr auto kAttrLabelId = "label_id"; +constexpr auto kAttrLogicId = "logic_id"; +constexpr auto kAttrNodeInfo = "node_info"; +constexpr auto kAttrNodeName = "node_name"; constexpr auto kAttrDynInput = "dynamic"; constexpr auto kAttrDynInputSizes = "dyn_input_sizes"; constexpr auto kAttrSrcFormat = "src_format";