-
Notifications
You must be signed in to change notification settings - Fork 18
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
libc: string.h: add cmp functions tests #199
Conversation
17631ea
to
d5a5ffe
Compare
d5a5ffe
to
8c25937
Compare
2b198ec
to
a6857a5
Compare
5aef5a0
to
f820b62
Compare
96eff61
to
67b8816
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for correcting the test to this version, it looks clear, but let's improve it a little bit.
3d33532
to
601ca2c
Compare
601ca2c
to
e7eb662
Compare
libc/string_cmp.c
Outdated
|
||
|
||
/* Loop which changes the size of compared space to place where asciSet ends */ | ||
for (i = 1; i < sizeof(asciiSet); i++) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It was not applied, I've suggested sth similar, but with the ascii data and with checking two sizes in one iteration.
e7eb662
to
b5fe230
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please also add a cases offsets
to the rest of tested functions. I know it wasn't planned initially, but I think it will be worth as the implementation of strcmp for example is very close to the memcmp
one. You would have to replace all of zeros with 1 for example when preparing test data for those functions and set NUL term (the rest of functions test strings).
b5fe230
to
9587a8d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small thing, but I think we should be consistent and start with a lowercase letter after suffix in commit message, so testdata: extend description for createCharStr
9587a8d
to
d48b474
Compare
d48b474
to
a06fe00
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, Let's wait with merge for #230
JIRA: CI-231
JIRA: CI-231
a06fe00
to
6f47132
Compare
Description
Added
function tests.
Motivation and Context
JIRA: CI-231
Types of changes
How Has This Been Tested?
Checklist:
Special treatment