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

Handle 0 and negative numbers on string_copy #1352

Merged
merged 2 commits into from
Apr 11, 2022
Merged

Conversation

Drosaca
Copy link
Contributor

@Drosaca Drosaca commented Mar 31, 2022

Originating Project/Creator
Affected Component
Affected Architectures(s)
Related Issue(s)
Has Unit Tests (y/n)
Builds Without Errors (y/n)
Unit Tests Pass (y/n)
Documentation Included (y/n)

Change Description

Avoids to write '\0' before the destination pointer if num is 0
which is set by the following line:

destination[ num - 1] = '\0'

@ThibFrgsGmz
Copy link
Contributor

The verification of the negative number seems useless because the number passed in parameter is not signed. Right ?

@Drosaca
Copy link
Contributor Author

Drosaca commented Apr 1, 2022

@ThibFrgsGmz Indeed the check of negative values has been removed, nevertheless when num equals zero '\0' will still be written at destination[-1]

@LeStarch
Copy link
Collaborator

LeStarch commented Apr 4, 2022

I agree with this improvement. In the case where 0 is supplied as a limiting length, then the destination should return unchanged as the limit does not even account for a terminating \0. This will will remove the error.

@LeStarch LeStarch closed this Apr 11, 2022
@LeStarch LeStarch reopened this Apr 11, 2022
@LeStarch
Copy link
Collaborator

Closed and reopened as I needed to try and re-trigger a complete rerun of CI to fix the python formatting bug. If it doesn't work, we'll ignore that check as the python was not touched.

@LeStarch LeStarch merged commit ea1b4d5 into nasa:devel Apr 11, 2022
LeStarch pushed a commit that referenced this pull request Jun 29, 2022
* Handle 0 and negative numbers on string_copy

* remove below zero check
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