Skip to content
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.

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

Fix compiler warnings related to strings #697

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

averater
Copy link
Contributor

Fix string truncation and overflow warnings so the compiler flags stringop-overflow and stringop-truncation can be enabled.

This shouldn't change any behaviour nor fix any bugs. It is just to be able to run the compiler with those flags so future bugs can be avoided. However I have added some extra checks for string lengths.

Most changes is to avoid using strncat and instead use strcat. This doesn't make the code safer but removes the false safeness of using strncat wrongly.

Fix string truncation and overflow warnings so the compiler flags
stringop-overflow and stringop-truncation can be enabled.

This shouldn't change any behaviour nor fix any bugs. It is just
to be able to run the compiler with those flags so future bugs
can be avoided. However I have added some extra checks for string
lengths.

Most changes is to avoid using strncat and instead use strcat. This
doesn't make the code safer but removes the false safeness of using
strncat wrongly.
@averater averater mentioned this pull request Oct 29, 2024
@@ -113,7 +113,7 @@ void set_node_id(char *id)
exit(-1);
}
else {
strncpy(g_options.node_id, id, DLT_ID_SIZE);
strcpy(g_options.node_id, id);
Copy link
Collaborator

Choose a reason for hiding this comment

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

This may overwrite the end of array g_options.node_id by one byte. node_id is not handled as a null terminated string.
strncpy is required to restrict the operation to 4 bytes. DLT IDs are not Null-terminated in dlt-daemon.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thank you, I have restored it. Since there is no length check of argv this is indeed needed.

Copy link
Collaborator

Choose a reason for hiding this comment

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

@michael-methner
Good point for no null-terminated for App and Context ID

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants