Skip to content

Fix compare of file suffix #703

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Fix compare of file suffix #703

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 5 additions & 3 deletions src/offlinelogstorage/dlt_offline_logstorage_behavior.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,11 +382,13 @@ int dlt_logstorage_storage_dir_info(DltLogStorageUserConfig *file_config,
if (strncmp(files[i]->d_name, file_name, len) == 0) {
if (config->num_files == 1 && file_config->logfile_optional_counter) {
/* <filename>.dlt or <filename>_<tmsp>.dlt */
if ((files[i]->d_name[len] == suffix[0]) ||
if (strcmp(files[i]->d_name + len, suffix) == 0 ||
Copy link
Collaborator

@lti9hc lti9hc Nov 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @averater
this comparation is wrong when the file contains timestamp

Copy link
Contributor Author

@averater averater Nov 21, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you mean it is wrong also as it is? Because all I did was change the suffix comparison from only comparing the "." to comparing the entire suffix.

If we need to change the comparison for files with timestamps then maybe that would be a different pull request?

Copy link
Collaborator

@lti9hc lti9hc Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@averater
I know, it should be covered all cases in your PR.
You can apply in current PR, don't need other PR

Copy link
Contributor Author

@averater averater Nov 25, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would prefer if someone else fixed that part as I have other task with higher prio and don't have time to look into this soon. Either merge only this PR and fix only this part or someone else has to fix all of it.

Copy link
Contributor Author

@averater averater Sep 16, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR fixes the case without timestamp. If there is a timestamp it will only check for a delimiter. Next line:
(file_config->logfile_timestamp &&
(files[i]->d_name[len] == file_config->logfile_delimiter))) {
To fix that we need to either check files[i]->d_nam from the back or know how long the timestamp is. That will be needed in this PR: #772

(file_config->logfile_timestamp &&
(files[i]->d_name[len] == file_config->logfile_delimiter))) {
(files[i]->d_name[len] ==
file_config->logfile_delimiter))) {
current_idx = 1;
} else {
}
else {
continue;
}
} else {
Expand Down
Loading