A bunch of safe string changes #22
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Do not build the test program as static. Static programs are not supported on some modern distros. relro is also rejected, so drop it. Signed-off-by: Mark Rustad <[email protected]>
Gcc9 complains about passing null string pointers, so resolve the obvious cases that the compiler reports by passing "" instead of the null pointer. Signed-off-by: Mark Rustad <[email protected]>
Many test failures only print when DEBUG is defined. This is crazy. Always print test failures. Also count test failures and return the failures count instead of uselessly returning 0 from each test routine. That allows the test to stop and clearly report the presence of errors. Signed-off-by: Mark Rustad <[email protected]>
…tignore Ignore more built items. Signed-off-by: Mark Rustad <[email protected]>
The return values from strcasestr and strcasestr_s were being cast to int for comparison. This is bogus because pointers are only guaranteed to fit in longs. I suspect that this was done due to compiler warnings. Those warnings were because strcasestr was compiled without a prototype because it is _GNU_SOURCE only, and that symbol was not defined. So without the prototype, the return value of strcasestr was an integer and not a pointer. I assume that the warnings were not enabled to reveal the missing prototype. Fix both problems. Signed-off-by: Mark Rustad <[email protected]>>
Casting a char * to a wchar_t makes no sense, especially when the value is supposed to be a wchar_t *. I don't know how this could even be working. Signed-off-by: Mark Rustad <[email protected]>
Include ctype.h and stdlib.h to get prototypes for functions referenced. This resolves implicit declaration warnings. Signed-off-by: Mark Rustad <[email protected]>
The code is checking the wrong variable when testing memcmp_s. This was found by compiling with -Wall and finding a set but unused variable. Also changed the type of the variable, since memcmp returns an int, not an int32_t. Signed-off-by: Mark Rustad <[email protected]>
Extremely long lines make patches hard to review. Shorten them by using simply expanded variables and appending to them. These variables have no need for recursive expansion. Signed-off-by: Mark Rustad <[email protected]>
… list The compiler generates perfectly good dependencies. Use them. Signed-off-by: Mark Rustad <[email protected]>
Turning on -Wall revealed some uninitialized variables, so fix them. Signed-off-by: Mark Rustad <[email protected]>
Turn on -Wall and resolve the remaining warning. This involved eliminating a set but unused variable. Signed-off-by: Mark Rustad <[email protected]>
Add the -Wextra flag and resolve the resulting warnings. These include marking unused parameters as unused, adding fallthrough comments switch statement cases that fall through and resolving sign comparison differences. Signed-off-by: Mark Rustad <[email protected]>
Function prototypes belong in a common header so that there is some check that the caller and callee are compatible. Move extern declarations out of the .c and into the private .h. Make them proper prototypes at the same time. Signed-off-by: Mark Rustad <[email protected]>
Add -W strict-prototypes and resolve the resulting warnings. Add static where necessary, and convert non-prototypes into prototypes. Signed-off-by: Mark Rustad <[email protected]>
Although the documentation describes destination buffers being filled with 0, that is only done when SAFECLIB_STR_NULL_SLACK is defined. Make the source documentation conditional on the same variable. Documentation outside the source is perpetually at risk of being different than how any particular library is built. A such, most documentation should assume that the clearing is not done, as that is the least risk assumption. Signed-off-by: Mark Rustad <[email protected]>
When SAFECLIB_STR_NULL_SLACK is defined, several unit tests fail. Some fail because the functions misbehave, others fail because the tests are expecting different values in the slack buffer. Fix them all. Signed-off-by: Mark Rustad <[email protected]>
Order-only dependencies work well for creating object directories, so use them. Also, remove those directories when doing a clean. Adds a missing .PHONY for the all rule. Signed-off-by: Mark Rustad <[email protected]>>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This is a series of changes to safestring to fix some test failures when SAFECLIB_STR_NULL_SLACK is defined, a variety of Makefile improvements, turning on more warnings and resolving them, adding conditionals to the function comments so that the NULL slack behavior is not presumed to be enabled (having the comments not match the function is evil). The documentation pages also should be changed, because they describe the NULL slack behavior even though that is not what the default library implementation actually does. Oddly, this make strncpy_s less secure, from an information leak standpoint than the standard strncpy!