Libcperciva import #296
Merged
Libcperciva import #296
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
We tried to support valgrind checking of fork() in:
2023-09-20 tests/valgrind: improve handling for fork() and exec()
38e07b49d7bc662fb19917f07d19176d22cf9b54
However, that method assumed that the lowest pid would be the parent,
while the higher one(s) would be the children. That has two flaws:
1) If the pid rolls over, the order would be broken. For example, if
the parent pid was 99999 on FreeBSD (the maximum value), then the
child would be something much lower (probably in the hundreds).
(I knew about that problem, considered it sufficiently unlikely.)
2) Since $_valgrind_check_logfiles are defined by `ls "blah"*`, they are
listed in lexicographical order, not numerical order. For example,
given two logfiles foo-9999.log and foo-10000.log, foo-10000.log
would come first.
(I did not anticipate this problem, but even if it had occurred to
me, I probably would have deemed it sufficiently unlikely. However,
by unlikely fluke, this seems to occur roughly 50% of the time with
github actions: when checking 24-fork-func with the gcc binaries, the
parent lands on pid 9999 exactly and the child is 10000.)
The fix is not to rely on the order: instead, generate a list of all
pids involved. If a valgrind logfile lists a parent pid which is in
that list, it's a child; otherwise, it's the parent.
For reference, see the "all:" target on line 26.
This should have been part of:
2023-04-08 build: add LIBS variable
978f267736bab1237e6097c5bd06a7bd40edb40a
and:
2018-06-28 build: add ability to generate top-level flag files
645d809f625a2f7f0e8c9553cc41987ffc73b228
netbuf:
- Agreement between API comments in header file and c files.
- Use () for function names.
tests/{crc32,crypto_aesctr}:
- clarify that we're not using random() for anything cryptographic.
(Static analyzers love to complain about this, so let's make it clear
to humans that this isn't a problem.)
Other:
- Remove unnecessary double spaces.
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.
No description provided.