From acbbc6b23d6694ee940ad384bf882a4c4ab47af7 Mon Sep 17 00:00:00 2001 From: Bui Nguyen Quoc Thanh Date: Mon, 18 May 2020 14:52:05 +0700 Subject: offline storage: Improvement log messages at bottom - In case there are several filters in config dlt_logstorage.conf which are sharing the same file name, the dlt-daemon could not handle to write the dlt messages at the proper file. It means the latest dlt messages might be not written into the newest file. - In order to resolve this, dlt-daemon must keep the newest file for each filename. Everytime new dlt message is going to be written, dlt-daemon must ensure that it must be written out to the newest file. Signed-off-by: Bui Nguyen Quoc Thanh Signed-off-by: KHANH LUONG HONG DUY --- src/offlinelogstorage/dlt_offline_logstorage.c | 139 +++++++++++++++++---- src/offlinelogstorage/dlt_offline_logstorage.h | 17 ++- .../dlt_offline_logstorage_behavior.c | 83 ++++++++---- .../dlt_offline_logstorage_behavior.h | 6 +- .../dlt_offline_logstorage_behavior_internal.h | 3 +- 5 files changed, 195 insertions(+), 53 deletions(-) diff --git a/src/offlinelogstorage/dlt_offline_logstorage.c b/src/offlinelogstorage/dlt_offline_logstorage.c index 7bc7bfd..374bbb6 100644 --- a/src/offlinelogstorage/dlt_offline_logstorage.c +++ b/src/offlinelogstorage/dlt_offline_logstorage.c @@ -45,12 +45,25 @@ DLT_STATIC void dlt_logstorage_filter_config_free(DltLogStorageFilterConfig *dat DltLogStorageFileList *n = NULL; DltLogStorageFileList *n1 = NULL; - free(data->apids); - data->apids = NULL; - free(data->ctids); - data->ctids = NULL; - free(data->file_name); - data->file_name = NULL; + if (data->apids) { + free(data->apids); + data->apids = NULL; + } + + if (data->ctids) { + free(data->ctids); + data->ctids = NULL; + } + + if (data->file_name) { + free(data->file_name); + data->file_name = NULL; + } + + if (data->working_file_name) { + free(data->working_file_name); + data->working_file_name = NULL; + } if (data->ecuid != NULL) { free(data->ecuid); @@ -70,8 +83,11 @@ DLT_STATIC void dlt_logstorage_filter_config_free(DltLogStorageFilterConfig *dat while (n) { n1 = n; n = n->next; - free(n1->name); - n1->name = NULL; + if (n1->name) { + free(n1->name); + n1->name = NULL; + } + free(n1); n1 = NULL; } @@ -131,14 +147,6 @@ DLT_STATIC int dlt_logstorage_list_add_config(DltLogStorageFilterConfig *data, if (*(listdata) == NULL) return -1; - (*listdata)->apids = NULL; - (*listdata)->ctids = NULL; - (*listdata)->file_name = NULL; - (*listdata)->ecuid = NULL; - (*listdata)->records = NULL; - (*listdata)->log = NULL; - (*listdata)->cache = NULL; - /* copy the data to list */ memcpy(*listdata, data, sizeof(DltLogStorageFilterConfig)); @@ -724,7 +732,10 @@ DLT_STATIC int dlt_logstorage_prepare_table(DltLogStorage *handle, { int ret = 0; int num_keys = 0; + int found = 0; char *keys = NULL; + DltNewestFileName *tmp = NULL; + DltNewestFileName *new_tmp = NULL; if ((handle == NULL) || (data == NULL)) { dlt_vlog(LOG_ERR, "Invalid parameters in %s\n", __func__); @@ -755,6 +766,42 @@ DLT_STATIC int dlt_logstorage_prepare_table(DltLogStorage *handle, return -1; } + if (data->file_name) { + if (handle->newest_file_list != NULL) { + tmp = handle->newest_file_list; + while (tmp) { + if (strcmp(tmp->file_name, data->file_name) == 0) { + found = 1; + break; + } + else { + tmp = tmp->next; + } + } + } + + if (!found) { + new_tmp = calloc(1, sizeof(DltNewestFileName)); + if (new_tmp == NULL) { + /* In this case, the existing list does not need to be freed.*/ + dlt_vlog(LOG_ERR, + "Failed to allocate memory for new file name [%s]\n", + data->file_name); + free(keys); + keys = NULL; + return -1; + } + new_tmp->file_name = strdup(data->file_name); + new_tmp->newest_file = NULL; + new_tmp->next = NULL; + + if (handle->newest_file_list == NULL) + handle->newest_file_list = new_tmp; + else + tmp->next = new_tmp; + } + } + free(keys); keys = NULL; return 0; @@ -1392,15 +1439,8 @@ DLT_STATIC int dlt_daemon_offline_setup_filter_properties(DltLogStorage *handle, return DLT_OFFLINE_LOGSTORAGE_STORE_FILTER_ERROR; memset(&tmp_data, 0, sizeof(DltLogStorageFilterConfig)); - tmp_data.apids = NULL; - tmp_data.ctids = NULL; - tmp_data.file_name = NULL; - tmp_data.ecuid = NULL; tmp_data.log_level = DLT_LOG_VERBOSE; tmp_data.reset_log_level = DLT_LOG_OFF; - tmp_data.records = NULL; - tmp_data.log = NULL; - tmp_data.cache = NULL; for (i = 0; i < DLT_LOGSTORAGE_FILTER_CONF_COUNT; i++) { ret = dlt_logstorage_get_filter_value(config_file, sec_name, i, value); @@ -1430,6 +1470,11 @@ DLT_STATIC int dlt_daemon_offline_setup_filter_properties(DltLogStorage *handle, tmp_data.file_name = NULL; } + if (tmp_data.working_file_name != NULL) { + free(tmp_data.working_file_name); + tmp_data.working_file_name = NULL; + } + if (tmp_data.ecuid != NULL) { free(tmp_data.ecuid); tmp_data.ecuid = NULL; @@ -1627,6 +1672,7 @@ int dlt_logstorage_device_connected(DltLogStorage *handle, char *mount_point) handle->config_status = 0; handle->write_errors = 0; handle->num_configs = 0; + handle->newest_file_list = NULL; /* Setup logstorage with config file settings */ return dlt_logstorage_load_config(handle); @@ -1644,6 +1690,7 @@ int dlt_logstorage_device_connected(DltLogStorage *handle, char *mount_point) */ int dlt_logstorage_device_disconnected(DltLogStorage *handle, int reason) { + DltNewestFileName *tmp = NULL; if (handle == NULL) return -1; @@ -1658,6 +1705,21 @@ int dlt_logstorage_device_disconnected(DltLogStorage *handle, int reason) handle->write_errors = 0; handle->num_configs = 0; + while (handle->newest_file_list) { + tmp = handle->newest_file_list; + handle->newest_file_list = tmp->next; + if (tmp->file_name) { + free(tmp->file_name); + tmp->file_name = NULL; + } + if (tmp->newest_file) { + free(tmp->newest_file); + tmp->newest_file = NULL; + } + free(tmp); + tmp = NULL; + } + return 0; } @@ -1915,7 +1977,7 @@ DLT_STATIC int dlt_logstorage_filter(DltLogStorage *handle, * configuration. * * @param handle DltLogStorage handle - * @param uconfig User configurations for log file + * @param uconfig User configurations for log file * @param data1 Data buffer of message header * @param size1 Size of message header buffer * @param data2 Data buffer of extended message body @@ -1946,6 +2008,8 @@ int dlt_logstorage_write(DltLogStorage *handle, DltStandardHeader *standardHeader = NULL; unsigned int standardHeaderExtraLen = sizeof(DltStandardHeaderExtra); unsigned int header_len = 0; + DltNewestFileName *tmp = NULL; + int found = 0; int log_level = -1; @@ -2027,11 +2091,36 @@ int dlt_logstorage_write(DltLogStorage *handle, if (config[i]->file_name == NULL) continue; + tmp = handle->newest_file_list; + while (tmp) { + if (strcmp(tmp->file_name, config[i]->file_name) == 0) { + found = 1; + break; + } + else { + tmp = tmp->next; + } + } + if (!found) { + dlt_vlog(LOG_ERR, "Cannot find out record for filename [%s]\n", + config[i]->file_name); + return -1; + } + /* prepare log file (create and/or open)*/ ret = config[i]->dlt_logstorage_prepare(config[i], uconfig, handle->device_mount_point, - size1 + size2 + size3); + size1 + size2 + size3, + tmp->newest_file); + if (tmp->newest_file == NULL || + strcmp(config[i]->working_file_name, tmp->newest_file) != 0) { + if (tmp->newest_file) { + free(tmp->newest_file); + tmp->newest_file = NULL; + } + tmp->newest_file = strdup(config[i]->working_file_name); + } if (ret == 0) { /* log data (write) */ ret = config[i]->dlt_logstorage_write(config[i], diff --git a/src/offlinelogstorage/dlt_offline_logstorage.h b/src/offlinelogstorage/dlt_offline_logstorage.h index 1f7396d..b58da70 100644 --- a/src/offlinelogstorage/dlt_offline_logstorage.h +++ b/src/offlinelogstorage/dlt_offline_logstorage.h @@ -52,6 +52,7 @@ #define DLT_OFFLINE_LOGSTORAGE_H #include +#include #include "dlt_common.h" #include "dlt-daemon_cfg.h" #include "dlt_config_file_parser.h" @@ -139,7 +140,7 @@ typedef struct DltLogStorageFileList /* List for filenames */ char *name; /* Filename */ unsigned int idx; /* File index */ - struct DltLogStorageFileList *next; + struct DltLogStorageFileList *next; /* Pointer to next */ } DltLogStorageFileList; typedef struct DltLogStorageFilterConfig DltLogStorageFilterConfig; @@ -152,6 +153,7 @@ struct DltLogStorageFilterConfig int log_level; /* Log level number configured for filter */ int reset_log_level; /* reset Log level to be sent on disconnect */ char *file_name; /* File name for log storage configured for filter */ + char *working_file_name; /* Current open log file name */ unsigned int file_size; /* MAX File size of storage file configured for filter */ unsigned int num_files; /* MAX number of storage files configured for filters */ int sync; /* Sync strategy */ @@ -160,7 +162,8 @@ struct DltLogStorageFilterConfig int (*dlt_logstorage_prepare)(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int log_msg_size); + int log_msg_size, + char *newest_file); int (*dlt_logstorage_write)(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, @@ -193,6 +196,15 @@ struct DltLogStorageFilterList DltLogStorageFilterList *next; /* Pointer to next */ }; +typedef struct DltNewestFileName DltNewestFileName; + +struct DltNewestFileName +{ + char *file_name; /* The unique name of file in whole a dlt_logstorage.conf */ + char *newest_file; /* The real newest name of file which is associated with filename.*/ + DltNewestFileName *next; /* Pointer to next */ +}; + typedef struct { DltLogStorageFilterList *config_list; /* List of all filters */ @@ -202,6 +214,7 @@ typedef struct unsigned int connection_type; /* Type of connection */ unsigned int config_status; /* Status of configuration */ int write_errors; /* number of write errors */ + DltNewestFileName *newest_file_list; /* List of newest file name */ } DltLogStorage; typedef struct { diff --git a/src/offlinelogstorage/dlt_offline_logstorage_behavior.c b/src/offlinelogstorage/dlt_offline_logstorage_behavior.c index 7c55a1a..5f5c164 100644 --- a/src/offlinelogstorage/dlt_offline_logstorage_behavior.c +++ b/src/offlinelogstorage/dlt_offline_logstorage_behavior.c @@ -269,6 +269,8 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, int ret = 0; struct dirent **files = { 0 }; unsigned int current_idx = 0; + DltLogStorageFileList *n = NULL; + DltLogStorageFileList *n1 = NULL; if ((config == NULL) || (file_config == NULL) || @@ -284,13 +286,27 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, return -1; } + /* In order to have a latest status of file list, + * the existing records must be deleted before updating + */ + n = config->records; + if (config->records) { + while (n) { + n1 = n; + n = n->next; + free(n1->name); + n1->name = NULL; + free(n1); + n1 = NULL; + } + config->records = NULL; + } + for (i = 0; i < cnt; i++) { int len = 0; len = strlen(config->file_name); - if ((strncmp(files[i]->d_name, - config->file_name, - len) == 0) && + if ((strncmp(files[i]->d_name, config->file_name, len) == 0) && (files[i]->d_name[len] == file_config->logfile_delimiter)) { DltLogStorageFileList **tmp = NULL; current_idx = dlt_logstorage_get_idx_of_log_file(file_config, @@ -301,8 +317,7 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, if (config->records == NULL) { ret = -1; - dlt_log(LOG_ERR, - "Memory allocation failed\n"); + dlt_log(LOG_ERR, "Memory allocation failed\n"); break; } @@ -318,8 +333,7 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, if (*tmp == NULL) { ret = -1; - dlt_log(LOG_ERR, - "Memory allocation failed\n"); + dlt_log(LOG_ERR, "Memory allocation failed\n"); break; } } @@ -356,12 +370,14 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, * @param file_config User configurations for log file * @param dev_path Storage device path * @param msg_size Size of incoming message + * @param is_update_required The file list needs to be updated * @return 0 on succes, -1 on error */ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int msg_size) + int msg_size, + bool is_update_required) { int ret = 0; char absolute_file_path[DLT_MOUNT_PATH_MAX + DLT_OFFLINE_LOGSTORAGE_CONFIG_DIR_PATH_LEN + 1] = { '\0' }; @@ -383,9 +399,8 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, snprintf(storage_path, DLT_OFFLINE_LOGSTORAGE_CONFIG_DIR_PATH_LEN, "%s/", dev_path); /* check if there are already files stored */ - if (config->records == NULL) { - if (dlt_logstorage_storage_dir_info(file_config, storage_path, config) - != 0) + if (config->records == NULL || is_update_required) { + if (dlt_logstorage_storage_dir_info(file_config, storage_path, config) != 0) return -1; } @@ -411,14 +426,14 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, /* concatenate path and file and open absolute path */ strcat(absolute_file_path, storage_path); strcat(absolute_file_path, file_name); + config->working_file_name = strdup(file_name); config->log = fopen(absolute_file_path, "a+"); /* Add file to file list */ *tmp = malloc(sizeof(DltLogStorageFileList)); if (*tmp == NULL) { - dlt_log(LOG_ERR, - "Memory allocation for file name failed\n"); + dlt_log(LOG_ERR, "Memory allocation for file name failed\n"); return -1; } @@ -430,6 +445,13 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, strcat(absolute_file_path, storage_path); strcat(absolute_file_path, (*newest)->name); + if (config->working_file_name != NULL) { + free(config->working_file_name); + config->working_file_name = NULL; + } + + config->working_file_name = strdup((*newest)->name); + ret = stat(absolute_file_path, &s); /* if size is enough, open it */ @@ -460,14 +482,19 @@ int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, sizeof(absolute_file_path) / sizeof(char)); strcat(absolute_file_path, storage_path); strcat(absolute_file_path, file_name); + + if(config->working_file_name) { + free(config->working_file_name); + config->working_file_name = strdup(file_name); + } + config->log = fopen(absolute_file_path, "a+"); /* Add file to file list */ *tmp = malloc(sizeof(DltLogStorageFileList)); if (*tmp == NULL) { - dlt_log(LOG_ERR, - "Memory allocation for file name failed\n"); + dlt_log(LOG_ERR, "Memory allocation for file name failed\n"); return -1; } @@ -642,7 +669,7 @@ DLT_STATIC int dlt_logstorage_sync_to_file(DltLogStorageFilterConfig *config, if (config->log == NULL) { if (dlt_logstorage_prepare_on_msg(config, file_config, dev_path, - config->file_size) != 0) + config->file_size, NULL) != 0) { dlt_vlog(LOG_ERR, "%s: failed to prepare log file\n", __func__); @@ -681,7 +708,7 @@ DLT_STATIC int dlt_logstorage_sync_to_file(DltLogStorageFilterConfig *config, if (config->log == NULL) { if (dlt_logstorage_prepare_on_msg(config, file_config, dev_path, - config->file_size) != 0) + config->file_size, NULL) != 0) { dlt_vlog(LOG_ERR, "%s: failed to prepare log file\n", __func__); return -1; @@ -711,37 +738,44 @@ DLT_STATIC int dlt_logstorage_sync_to_file(DltLogStorageFilterConfig *config, * @param file_config User configurations for log file * @param dev_path Storage device path * @param log_msg_size Size of log message + * @param newest_file Name of newest file for corresponding filename * @return 0 on success, -1 on error */ int dlt_logstorage_prepare_on_msg(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int log_msg_size) + int log_msg_size, + char *newest_file) { int ret = 0; struct stat s; - if ((config == NULL) || (file_config == NULL) || (dev_path == NULL)) + if ((config == NULL) || (file_config == NULL) || (dev_path == NULL)) { + dlt_vlog(LOG_INFO, "Wrong paratemters\n"); return -1; + } if (config->log == NULL) { /* open a new log file */ ret = dlt_logstorage_open_log_file(config, file_config, dev_path, - log_msg_size); + log_msg_size, + true); } else { /* already open, check size and create a new file if needed */ ret = fstat(fileno(config->log), &s); if (ret == 0) { /* check if adding new data do not exceed max file size */ - if (s.st_size + log_msg_size > (int)config->file_size) { + if ((s.st_size + log_msg_size > (int)config->file_size) || + strcmp(config->working_file_name, newest_file) != 0) { fclose(config->log); config->log = NULL; ret = dlt_logstorage_open_log_file(config, file_config, dev_path, - log_msg_size); + log_msg_size, + true); } else { /*everything is prepared */ ret = 0; @@ -854,13 +888,16 @@ int dlt_logstorage_sync_on_msg(DltLogStorageFilterConfig *config, * @param file_config User configurations for log file * @param dev_path Storage device path * @param log_msg_size Size of log message + * @param newest_file Name of newest file for corresponding filename * @return 0 on success, -1 on error */ int dlt_logstorage_prepare_msg_cache(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int log_msg_size) + int log_msg_size, + char *newest_file ) { + (void) newest_file; if ((config == NULL) || (file_config == NULL) || (dev_path == NULL)) return -1; diff --git a/src/offlinelogstorage/dlt_offline_logstorage_behavior.h b/src/offlinelogstorage/dlt_offline_logstorage_behavior.h index 34305d0..cb94aa7 100644 --- a/src/offlinelogstorage/dlt_offline_logstorage_behavior.h +++ b/src/offlinelogstorage/dlt_offline_logstorage_behavior.h @@ -54,7 +54,8 @@ int dlt_logstorage_prepare_on_msg(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int log_msg_size); + int log_msg_size, + char *newest_file); int dlt_logstorage_write_on_msg(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, @@ -76,7 +77,8 @@ int dlt_logstorage_sync_on_msg(DltLogStorageFilterConfig *config, int dlt_logstorage_prepare_msg_cache(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int log_msg_size); + int log_msg_size, + char *newest_file); int dlt_logstorage_write_msg_cache(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, diff --git a/src/offlinelogstorage/dlt_offline_logstorage_behavior_internal.h b/src/offlinelogstorage/dlt_offline_logstorage_behavior_internal.h index f386ea7..c059fbd 100644 --- a/src/offlinelogstorage/dlt_offline_logstorage_behavior_internal.h +++ b/src/offlinelogstorage/dlt_offline_logstorage_behavior_internal.h @@ -68,7 +68,8 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config, int dlt_logstorage_open_log_file(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, char *dev_path, - int msg_size); + int msg_size, + bool is_update_required); DLT_STATIC int dlt_logstorage_sync_to_file(DltLogStorageFilterConfig *config, DltLogStorageUserConfig *file_config, -- cgit v1.2.1