removing unnecessary boiler plate in unit.c #65
Merged
Conversation
This file contains 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
binaryfusefilter.h only issues addressed 1. changed some return value and parameter types of (static) functions -- PLEASE CHECK THIS IN REVIEW 2. sprinkled 'U' into bitwise operations to silence warnings 3. casting to avoid "standard integer promotion rules" which resulted in signedness warnings 4. explicitly reducing results to the target type rather than letting it happen implicitly tests still passing
binaryfusefilter.h only issues addressed 1. changed some return value and parameter types of (static) functions -- PLEASE CHECK THIS IN REVIEW 2. sprinkled 'U' into bitwise operations to silence warnings 3. casting to avoid "standard integer promotion rules" which resulted in signedness warnings 4. explicitly reducing results to the target type rather than letting it happen implicitly 5. when and `if` statements ends in break or return, then a following `else if` can be just a new `if` tests still passing
mostly casting size_t down to uint32_t - maybe some internal struct types should have been size_t? also some integer promotion casts
mostly casting blocklengt to uint32_t to fit into keyindex.index should keyindex.index be a size_t?
very repetitive casting of mainly sizes and doubles.
with -Wconversion and -Wsign-conversion so putting these in the Makefile, so during "private" development with the Makefile new warnings will be noticed straight away but not in CMakeLists.txt, because as this is a header-only INTERFACE library, it would force these warning levels on the users.
turned up these additional 'U' tweaks
all sections were indentical except for the call to *contain() and *size_in_bytes some void* and function pointer juggling allowed to make this generic report code reduced by 2/3rds
for all but the special "failure rate test" the large function dispatch table is a litle annoying, but can be removed as well...TBC tests all pass
just a missing free()
remove initialization. it's not needed and compiler now happy
instead of having a wrapper function per action per filter type, we can cast the functions as a generic function pointer on the into the generic test runner, and then cast them as a compatible function pointer type inside the test runner. The generic `filter*` parameter cannot be `void*` and must be a dummy struct because: § 6.2.5..28: "All pointers to structure types shall have the same representation and alignment requirements as each other" (the same is not true for `void*` which may have different representation. This simple change results in a large code reduction and removes the unsightly and hard to remove array of boiler plate function wrappers.
Merged
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.
It turns that we don't need the large wall of function wrappers and can cast the function pointers on the way into
test()and then inside (in a similar way thatvoid*is used for pointer parameters).This works because §6.2.5...28 states: "All pointers to structure types shall have the same representation and alignment requirements as each other"
So we use a dummy
gen_filterstruct to represent all filter types.Also included is the removal of the last 2 testcases with the
_dupsuffix, because the same can be achieved by just reusing the previous 2 with a different value forrepeated_size.