remove functions that only reimplement standard C functions #103
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
mbhangui
approved these changes
Sep 12, 2019
Member
|
To the best of my knowledge, too (which is probably a weaker claim than yours). I'm wishing we could have more confidence than that in these changes, though, and thinking we could have it by following these steps:
|
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
from
15acd4d to
3f0f2bf
Compare
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
2 times, most recently
from
93ecc78 to
6da2d63
Compare
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
from
6da2d63 to
7bc0ee9
Compare
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
3 times, most recently
from
842f456 to
53d3b5a
Compare
Contributor
|
Looks all good, str_len now returns a size_t which generally is larger than unsigned int, but this should be truncated to an int fine (this will be another incoming patch...). |
leahneukirchen
approved these changes
May 18, 2020
josuah
approved these changes
May 19, 2020
josuah
left a comment
josuah
left a comment
There was a problem hiding this comment.
I doubt we will find any system that lack these basic <string.h> functions and still claim to support compiling C.
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
from
53d3b5a to
f473ad0
Compare
schmonz
requested changes
May 24, 2020
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
2 times, most recently
from
100090b to
577643d
Compare
Basically 2 things could happen here: -the custom implementation is used, which is probably slower than the one provided by the compiler -the compiler or linker notices that this is just strlen() and replaces it with the normal one In both cases we are better of just removing it altogether. It would in theory be possible that this implementation is faster than what your compiler or libc offers. But than your system is already slow, this few calls for your mailer daemon will not matter then anyway.
For the very same reasons as str_len() this can go away.
That's how that macro is defined, so use it.
This one is actually a bit special: byte_diff() was used only for implementing the byte_equal() macro and had only one direct callsite. And this in turn should have been calling byte_equal(), too. When touching it anyway change it to memcmp() directly.
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
from
577643d to
1d22422
Compare
schmonz
reviewed
May 24, 2020
schmonz
reviewed
May 24, 2020
This changes the return type, but noone ever looked at it anyway.
All of these functions are gone now and replaced by libc ones. This reverts commit 8654051.
DerDakon
force-pushed
the
Dakon-remove-std-functions
branch
from
1d22422 to
2bd5462
Compare
schmonz
approved these changes
May 24, 2020
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.
To the best of my knowledge all of these functions behave identical to their standard counterparts, so just use them instead.
In contrast to #65 this does not remove
byte_zero()as that is used to clear passwords in memory. Amemset()could be optimized out by the compiler as the value is not used afterwards anymore, so keeping that function has a slightly higher chance of getting things cleared. One could check for the existence ofexplicit_bzero()ormemset_s()and use that instead, but I think it overcomplicates things and just keeping it does no immediate harm. If we ever go to kill qmail-pop3d and qmail-popup we can go with a plain memset() then.