Fix precision issue on pcapng reader #1514
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
seladb
reviewed
Jul 30, 2024
There was a problem hiding this comment.
We add a // PCPP patch comment everywhere we change something in LightPcapNg so we can later hopefully contribute them to the original repo
There was a problem hiding this comment.
amended the commit
mserdarsanli
force-pushed
the
precision-fix
branch
from
aed5aa3 to
a47c054
Compare
seladb
reviewed
Aug 1, 2024
|
@mserdarsanli can you run If you don't know how to run it please let me know and I can run it for you |
The library persists timespec values correctly in the pcapng files, as it always uses nanosecond precision. But when reading pcapng file, to handle arbitrary precision, it uses floating point arithmetic and loses precision causing loaded tv_nsec values to be off up to 128 nanoseconds for timestamps around now.
mserdarsanli
force-pushed
the
precision-fix
branch
from
d3bb349 to
d598e00
Compare
|
Fixed the clang-format issue for that file |
seladb
approved these changes
Aug 12, 2024
|
Thank you @mserdarsanli for working on this fix, much appreciated! 🙏 🙏 |
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.
Added test shows the problem of lost precision caused by using doubles to convert 64-bit timestamps to tv_sec and tv_nsec values. Next commit is the fix making code use integer arithmetic and timestamps now load correctly.