Allow delimiter in filenames #772
Conversation
Allow delimiter (default:_) in file names. With this fix it is possile to have filters for logstorage with for example filename ECU and ECU_B. Signed-off-by: averater <[email protected]>
|
How this fix can affect the case? Can you give a brief explanation here? How idx plays here? |
There was a problem hiding this comment.
Pull Request Overview
This PR modifies the offline log storage behavior to allow delimiter characters (default: underscore) within filenames, enabling more flexible filter naming for log storage such as "ECU" and "ECU_B".
- Changes log level from ERROR to DEBUG when unable to calculate index from filename
- Adds logic to skip files with invalid index (index == 0) during directory scanning
- Improves robustness of log file parsing when delimiters appear in base filenames
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| if (idx == 0) | ||
| dlt_log(LOG_ERR, | ||
| "Unable to calculate index from log file name. Reset to 001.\n"); | ||
| dlt_log(LOG_DEBUG, "Unable to calculate index from log file name.\n"); |
There was a problem hiding this comment.
The error message should be more descriptive. Consider including the filename or providing more context about why the calculation failed, such as 'Unable to calculate index from log file name: %s' with the filename parameter.
| dlt_log(LOG_DEBUG, "Unable to calculate index from log file name.\n"); | |
| dlt_log(LOG_DEBUG, "Unable to calculate index from log file name: %s\n", file); |
If there are two filenames "ECU_.dlt" and "ECU_B_.dlt". Then when searching for idx of "ECU_.dlt" it will find "ECU_B_.dlt" and will try to parse "B_.dlt" to int. Since "B" is not possible to convert to int it will return 0. This means for "ECU_.dlt" the file counter will restart every time it checks the directory. |
|
And also if those files are not skipped (with continue) they are counted and deleted when max file counter is reached. |
| if (idx == 0) | ||
| dlt_log(LOG_ERR, | ||
| "Unable to calculate index from log file name. Reset to 001.\n"); | ||
| dlt_log(LOG_DEBUG, "Unable to calculate index from log file name.\n"); |
There was a problem hiding this comment.
by the way, could you correct the return value of dlt_logstorage_get_idx_of_log_file function
Currently, I saw below code
if (file_config == NULL || config == NULL || file == NULL)
return -1;
this function should not get -1, it should return 0 or greater
| current_idx = dlt_logstorage_get_idx_of_log_file(file_config, config, | ||
| files[i]->d_name); | ||
| if (current_idx == 0) { | ||
| continue; |
There was a problem hiding this comment.
@averater it is good to check here
could you check further case in dlt_logstorage_open_log_file function for below code
} else {
idx = dlt_logstorage_get_idx_of_log_file(file_config, config,
config->working_file_name);
}
There was a problem hiding this comment.
That call is unnecessary as it already has been done in dlt_logstorage_storage_dir_info. I'll remove it.
3 participants
Allow delimiter (default:_) in file names. With this fix it is possile to have filters for logstorage with for example filename ECU and ECU_B.