From aa48c40c8730c85154e776f70e799f1a6700218e Mon Sep 17 00:00:00 2001 From: Alexander Mohr Date: Sun, 15 Sep 2024 19:50:17 +0200 Subject: [PATCH] review: fix findings 2024/09/15 --- include/dlt/dlt_common.h | 3 - src/daemon/dlt-daemon.c | 231 --------------------------- src/daemon/dlt-daemon.h | 5 - src/shared/dlt_config_file_parser.c | 233 ++++++++++++++++++++++++++++ src/shared/dlt_config_file_parser.h | 9 ++ 5 files changed, 242 insertions(+), 239 deletions(-) diff --git a/include/dlt/dlt_common.h b/include/dlt/dlt_common.h index 42d95635..7c80288b 100644 --- a/include/dlt/dlt_common.h +++ b/include/dlt/dlt_common.h @@ -834,7 +834,6 @@ extern "C" #include /* For trace load control */ -#ifdef DLT_TRACE_LOAD_CTRL_ENABLE /* Number of slots in window for recording trace load (Default: 60) * Average trace load in this window will be used as trace load @@ -876,8 +875,6 @@ extern "C" */ #define DLT_TIMESTAMP_RESOLUTION (10000) -#endif - typedef struct { // Window for recording total bytes for each slots [bytes] diff --git a/src/daemon/dlt-daemon.c b/src/daemon/dlt-daemon.c index 1209022c..ad92b8b9 100644 --- a/src/daemon/dlt-daemon.c +++ b/src/daemon/dlt-daemon.c @@ -1073,237 +1073,6 @@ int app_id_default_log_level_config_parser(DltDaemon *daemon, } #endif -#ifdef DLT_TRACE_LOAD_CTRL_ENABLE -static bool is_ascii_only(const char *str) { - while (*str) { - if ((unsigned char)*str > 127) { - return false; - } - str++; - } - return true; -} - -/** - * Load configuration file parser - */ -int trace_load_config_file_parser(DltDaemon *daemon, DltDaemonLocal *daemon_local) -{ - FILE *pFile; - const int max_tokens = 4; - const int min_tokens = 3; - char tokens[max_tokens][value_length]; - char line[value_length - 1]; - char app_id_value[value_length]; - char ctx_id_value[value_length]; - char soft_limit_value[value_length]; - char hard_limit_value[value_length]; - int i; - uint32_t soft_limit; - uint32_t hard_limit; - - char *pch; - const char *filename; - - bool skipped; - - if (daemon->preconfigured_trace_load_settings != NULL) { - free(daemon->preconfigured_trace_load_settings); - daemon->preconfigured_trace_load_settings = NULL; - daemon->preconfigured_trace_load_settings_count = 0; - } - daemon->preconfigured_trace_load_settings = malloc(sizeof(DltTraceLoadSettings)); - - /* open configuration file */ - filename = daemon_local->flags.lvalue[0] - ? daemon_local->flags.lvalue - : CONFIGURATION_FILES_DIR "/dlt-trace-load.conf"; - - pFile = fopen (filename, "r"); - if (pFile == NULL) { - dlt_vlog(LOG_WARNING, "Cannot open trace load configuration file: %s\n", - filename); - return -errno; - } - - while (1) { - /* fetch line from configuration file */ - if (fgets(line, value_length - 1, pFile) == NULL) { - break; - } - - pch = strtok(line, " "); - app_id_value[0] = 0; - ctx_id_value[0] = 0; - soft_limit_value[0] = 0; - hard_limit_value[0] = 0; - soft_limit = 0U; - hard_limit = 0U; - memset(tokens, 0, sizeof(tokens)); - i = 0; - - skipped = false; - while (pch != NULL && i < max_tokens) { - /* ignore comments, empty lines and new lines */ - if (strncmp(pch, "#", 1) == 0 || strncmp(pch, "\n", 1) == 0 || - strncmp(pch, "\r", 1) == 0 || strncmp(pch, " ", 1) == 0) { - skipped = true; - break; - } - strncpy(tokens[i], pch, sizeof(tokens[i]) - 1); - pch = strtok(NULL, " "); - ++i; - } - - if (skipped && i < min_tokens) - continue; - - if (pch != NULL - && (pch[0] != '\n') - && (pch[0] != '\t') - && (pch[0] != ' ') - && (pch[0] != '#')) { - dlt_vlog(LOG_WARNING, - "Invalid trace load settings: too many tokens in line '%s'\n", line); - continue; - } - - bool has_ctx_id = i == max_tokens; - int soft_limit_idx = has_ctx_id ? 2 : 1; - int hard_limit_idx = has_ctx_id ? 3 : 2; - - strncpy(app_id_value, tokens[0], sizeof(app_id_value) - 1); - if ((strlen(app_id_value) == 0) - || (strlen(app_id_value) > DLT_ID_SIZE) - || (!is_ascii_only(app_id_value))) { - dlt_vlog(LOG_WARNING, - "Invalid apid for trace load settings: app id: '%s'\n", app_id_value); - continue; - } - - if (has_ctx_id) { - strncpy(ctx_id_value, tokens[1], sizeof(ctx_id_value) - 1); - if ((strlen(ctx_id_value) == 0) - || (strlen(ctx_id_value) > DLT_ID_SIZE) - || (!is_ascii_only(ctx_id_value))) { - dlt_vlog(LOG_WARNING, - "Invalid ctid for trace load settings: context id: '%s'\n", ctx_id_value); - continue; - } - } - - if (strlen(tokens[soft_limit_idx]) == 0) { - dlt_vlog(LOG_WARNING, - "Invalid soft_limit for trace load settings: app id: '%.4s', '%s'\n", - app_id_value, tokens[soft_limit_idx]); - continue; - } - - if (strlen(tokens[hard_limit_idx]) == 0) { - dlt_vlog(LOG_WARNING, - "Invalid hard_limit for trace load settings: app id: '%.4s', '%s'\n", - app_id_value, tokens[hard_limit_idx]); - continue; - } - - strncpy(soft_limit_value, tokens[soft_limit_idx], - sizeof(soft_limit_value) - 1); - strncpy(hard_limit_value, tokens[hard_limit_idx], - sizeof(hard_limit_value) - 1); - - errno = 0; - char *endptr; - endptr = NULL; - soft_limit = strtoul(soft_limit_value, &endptr, 10); - if ((errno != 0) - || ((soft_limit == 0) && (soft_limit_value[0] != '0')) - || (soft_limit_value[0] == '-') - || ((*endptr != '\n') && (*endptr != '\0'))) { - dlt_vlog(LOG_WARNING, - "Invalid soft_limit for trace load settings: app id: '%.4s', soft_limit '%s'\n", - app_id_value, soft_limit_value); - continue; - } - - errno = 0; - endptr = NULL; - hard_limit = strtoul(hard_limit_value, &endptr, 10); - if ((errno != 0) - || ((hard_limit == 0) && (hard_limit_value[0] != '0')) - || (hard_limit_value[0] == '-') - || ((*endptr != '\n') && (*endptr != '\0'))) { - dlt_vlog(LOG_WARNING, - "Invalid hard_limit for trace load settings: app id: '%.4s', hard_limit '%s'\n", - app_id_value, hard_limit_value); - continue; - } - - if (soft_limit > hard_limit) { - dlt_vlog(LOG_WARNING, - "Invalid trace load settings: app id: '%.4s', soft limit %u is greater than hard limit %u\n", - app_id_value, soft_limit, hard_limit); - continue; - } - - DltTraceLoadSettings *settings = NULL; - int num_settings = 0; - DltReturnValue rv = dlt_daemon_find_preconfigured_trace_load_settings( - daemon, app_id_value, ctx_id_value, &settings, &num_settings, - 0); - if (rv != DLT_RETURN_OK || num_settings != 0) { - dlt_vlog(LOG_WARNING, - "App id '%.4s' is already configured, or an error occurred, skipping entry\n", - app_id_value); - if (settings != NULL) { - free(settings); - } - continue; - } - - /* allocate one more element in the trace load settings */ - DltTraceLoadSettings *tmp = - realloc(daemon->preconfigured_trace_load_settings, - (++daemon->preconfigured_trace_load_settings_count) * - sizeof(DltTraceLoadSettings)); - - if (tmp == NULL) { - dlt_log(LOG_CRIT, - "Failed to allocate memory for trace load settings\n"); - return DLT_RETURN_ERROR; - } - - daemon->preconfigured_trace_load_settings = tmp; - settings = &daemon->preconfigured_trace_load_settings - [daemon->preconfigured_trace_load_settings_count - 1]; - memset(settings, 0, sizeof(DltTraceLoadSettings)); - settings->soft_limit = soft_limit; - settings->hard_limit = hard_limit; - - memcpy(settings->apid, app_id_value, DLT_ID_SIZE); - if (has_ctx_id) { - memcpy(settings->ctid, ctx_id_value, DLT_ID_SIZE); - dlt_vlog(LOG_INFO, - "Configured trace limits for app id '%.4s', ctx id '%.4s', soft limit: %u, hard_limit: %u\n", - app_id_value, ctx_id_value, soft_limit, hard_limit); - } else { - dlt_vlog(LOG_INFO, - "Configured trace limits for app id '%.4s', soft limit: %u, hard_limit: %u\n", - app_id_value, soft_limit, hard_limit); - } - - - - } /* while */ - fclose(pFile); - - // sort limits to improve search performance - qsort(daemon->preconfigured_trace_load_settings, daemon->preconfigured_trace_load_settings_count, - sizeof(DltTraceLoadSettings), - dlt_daemon_compare_trace_load_settings); - return 0; -} -#endif - static int dlt_mkdir_recursive(const char *dir) { int ret = 0; diff --git a/src/daemon/dlt-daemon.h b/src/daemon/dlt-daemon.h index 33e3efaf..d5d32cf4 100644 --- a/src/daemon/dlt-daemon.h +++ b/src/daemon/dlt-daemon.h @@ -292,11 +292,6 @@ int dlt_daemon_close_socket(int sock, DltDaemon *daemon, DltDaemonLocal *daemon_ #ifdef DLT_TRACE_LOAD_CTRL_ENABLE bool trace_load_keep_message( DltDaemonApplication *app, int size, DltDaemon *daemon, DltDaemonLocal *daemon_local, int verbose); -// Functions that are only exposed for testing and should not be public -// for normal builds -#ifdef DLT_UNIT_TESTS -int trace_load_config_file_parser(DltDaemon *daemon, DltDaemonLocal *daemon_local); -#endif #endif #endif /* DLT_DAEMON_H */ diff --git a/src/shared/dlt_config_file_parser.c b/src/shared/dlt_config_file_parser.c index 26c92f86..a8a12f26 100644 --- a/src/shared/dlt_config_file_parser.c +++ b/src/shared/dlt_config_file_parser.c @@ -567,3 +567,236 @@ int dlt_config_file_check_section_name_exists(const DltConfigFile *file, return 0; } + + +#ifdef DLT_TRACE_LOAD_CTRL_ENABLE +static bool is_ascii_only(const char *str) { + while (*str) { + if ((unsigned char)*str > 127) { + return false; + } + str++; + } + return true; +} + +/** + * Load configuration file parser + */ +int trace_load_config_file_parser(DltDaemon *daemon, DltDaemonLocal *daemon_local) +{ + FILE *pFile; + const int max_tokens = 4; + const int min_tokens = 3; + char tokens[max_tokens][value_length]; + char line[value_length - 1]; + char app_id_value[value_length]; + char ctx_id_value[value_length]; + char soft_limit_value[value_length]; + char hard_limit_value[value_length]; + int i; + uint32_t soft_limit; + uint32_t hard_limit; + + char *pch; + const char *filename; + + bool skipped; + + if (daemon->preconfigured_trace_load_settings != NULL) { + free(daemon->preconfigured_trace_load_settings); + daemon->preconfigured_trace_load_settings = NULL; + daemon->preconfigured_trace_load_settings_count = 0; + } + daemon->preconfigured_trace_load_settings = malloc(sizeof(DltTraceLoadSettings)); + + /* open configuration file */ + filename = daemon_local->flags.lvalue[0] + ? daemon_local->flags.lvalue + : CONFIGURATION_FILES_DIR "/dlt-trace-load.conf"; + + pFile = fopen (filename, "r"); + if (pFile == NULL) { + dlt_vlog(LOG_WARNING, "Cannot open trace load configuration file: %s\n", + filename); + return -errno; + } + + while (1) { + /* fetch line from configuration file */ + if (fgets(line, value_length - 1, pFile) == NULL) { + break; + } + + pch = strtok(line, " "); + app_id_value[0] = 0; + ctx_id_value[0] = 0; + soft_limit_value[0] = 0; + hard_limit_value[0] = 0; + soft_limit = 0U; + hard_limit = 0U; + memset(tokens, 0, sizeof(tokens)); + i = 0; + + skipped = false; + while (pch != NULL && i < max_tokens) { + /* ignore comments, empty lines and new lines */ + if (strncmp(pch, "#", 1) == 0 || strncmp(pch, "\n", 1) == 0 || + strncmp(pch, "\r", 1) == 0 || strncmp(pch, " ", 1) == 0) { + skipped = true; + break; + } + strncpy(tokens[i], pch, sizeof(tokens[i]) - 1); + pch = strtok(NULL, " "); + ++i; + } + + if (skipped && i < min_tokens) + continue; + + if (pch != NULL + && (pch[0] != '\n') + && (pch[0] != '\t') + && (pch[0] != ' ') + && (pch[0] != '#')) { + dlt_vlog(LOG_WARNING, + "Invalid trace load settings: too many tokens in line '%s'\n", line); + continue; + } + + bool has_ctx_id = i == max_tokens; + int soft_limit_idx = has_ctx_id ? 2 : 1; + int hard_limit_idx = has_ctx_id ? 3 : 2; + + strncpy(app_id_value, tokens[0], sizeof(app_id_value) - 1); + if ((strlen(app_id_value) == 0) + || (strlen(app_id_value) > DLT_ID_SIZE) + || (!is_ascii_only(app_id_value))) { + dlt_vlog(LOG_WARNING, + "Invalid apid for trace load settings: app id: '%s'\n", app_id_value); + continue; + } + + if (has_ctx_id) { + strncpy(ctx_id_value, tokens[1], sizeof(ctx_id_value) - 1); + if ((strlen(ctx_id_value) == 0) + || (strlen(ctx_id_value) > DLT_ID_SIZE) + || (!is_ascii_only(ctx_id_value))) { + dlt_vlog(LOG_WARNING, + "Invalid ctid for trace load settings: context id: '%s'\n", ctx_id_value); + continue; + } + } + + if (strlen(tokens[soft_limit_idx]) == 0) { + dlt_vlog(LOG_WARNING, + "Invalid soft_limit for trace load settings: app id: '%.4s', '%s'\n", + app_id_value, tokens[soft_limit_idx]); + continue; + } + + if (strlen(tokens[hard_limit_idx]) == 0) { + dlt_vlog(LOG_WARNING, + "Invalid hard_limit for trace load settings: app id: '%.4s', '%s'\n", + app_id_value, tokens[hard_limit_idx]); + continue; + } + + strncpy(soft_limit_value, tokens[soft_limit_idx], + sizeof(soft_limit_value) - 1); + strncpy(hard_limit_value, tokens[hard_limit_idx], + sizeof(hard_limit_value) - 1); + + errno = 0; + char *endptr; + endptr = NULL; + soft_limit = strtoul(soft_limit_value, &endptr, 10); + if ((errno != 0) + || ((soft_limit == 0) && (soft_limit_value[0] != '0')) + || (soft_limit_value[0] == '-') + || ((*endptr != '\n') && (*endptr != '\0'))) { + dlt_vlog(LOG_WARNING, + "Invalid soft_limit for trace load settings: app id: '%.4s', soft_limit '%s'\n", + app_id_value, soft_limit_value); + continue; + } + + errno = 0; + endptr = NULL; + hard_limit = strtoul(hard_limit_value, &endptr, 10); + if ((errno != 0) + || ((hard_limit == 0) && (hard_limit_value[0] != '0')) + || (hard_limit_value[0] == '-') + || ((*endptr != '\n') && (*endptr != '\0'))) { + dlt_vlog(LOG_WARNING, + "Invalid hard_limit for trace load settings: app id: '%.4s', hard_limit '%s'\n", + app_id_value, hard_limit_value); + continue; + } + + if (soft_limit > hard_limit) { + dlt_vlog(LOG_WARNING, + "Invalid trace load settings: app id: '%.4s', soft limit %u is greater than hard limit %u\n", + app_id_value, soft_limit, hard_limit); + continue; + } + + DltTraceLoadSettings *settings = NULL; + int num_settings = 0; + DltReturnValue rv = dlt_daemon_find_preconfigured_trace_load_settings( + daemon, app_id_value, ctx_id_value, &settings, &num_settings, + 0); + if (rv != DLT_RETURN_OK || num_settings != 0) { + dlt_vlog(LOG_WARNING, + "App id '%.4s' is already configured, or an error occurred, skipping entry\n", + app_id_value); + if (settings != NULL) { + free(settings); + } + continue; + } + + /* allocate one more element in the trace load settings */ + DltTraceLoadSettings *tmp = + realloc(daemon->preconfigured_trace_load_settings, + (++daemon->preconfigured_trace_load_settings_count) * + sizeof(DltTraceLoadSettings)); + + if (tmp == NULL) { + dlt_log(LOG_CRIT, + "Failed to allocate memory for trace load settings\n"); + return DLT_RETURN_ERROR; + } + + daemon->preconfigured_trace_load_settings = tmp; + settings = &daemon->preconfigured_trace_load_settings + [daemon->preconfigured_trace_load_settings_count - 1]; + memset(settings, 0, sizeof(DltTraceLoadSettings)); + settings->soft_limit = soft_limit; + settings->hard_limit = hard_limit; + + memcpy(settings->apid, app_id_value, DLT_ID_SIZE); + if (has_ctx_id) { + memcpy(settings->ctid, ctx_id_value, DLT_ID_SIZE); + dlt_vlog(LOG_INFO, + "Configured trace limits for app id '%.4s', ctx id '%.4s', soft limit: %u, hard_limit: %u\n", + app_id_value, ctx_id_value, soft_limit, hard_limit); + } else { + dlt_vlog(LOG_INFO, + "Configured trace limits for app id '%.4s', soft limit: %u, hard_limit: %u\n", + app_id_value, soft_limit, hard_limit); + } + + + + } /* while */ + fclose(pFile); + + // sort limits to improve search performance + qsort(daemon->preconfigured_trace_load_settings, daemon->preconfigured_trace_load_settings_count, + sizeof(DltTraceLoadSettings), + dlt_daemon_compare_trace_load_settings); + return 0; +} +#endif + diff --git a/src/shared/dlt_config_file_parser.h b/src/shared/dlt_config_file_parser.h index 1de8b009..bfecbce2 100644 --- a/src/shared/dlt_config_file_parser.h +++ b/src/shared/dlt_config_file_parser.h @@ -160,4 +160,13 @@ int dlt_config_file_get_value(const DltConfigFile *file, */ int dlt_config_file_check_section_name_exists(const DltConfigFile *file, const char *name); + +#ifdef DLT_TRACE_LOAD_CTRL_ENABLE +// Functions that are only exposed for testing and should not be public +// for normal builds +#ifdef DLT_UNIT_TESTS +int trace_load_config_file_parser(DltDaemon *daemon, DltDaemonLocal *daemon_local); +#endif +#endif + #endif