-
Notifications
You must be signed in to change notification settings - Fork 38.6k
refactor: Fix implicit value conversion in formatPingTime #18260
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
Concept ACK |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsNo conflicts as of last run. |
hebasto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Concept ACK.
|
cc @vasild |
Note the divisor is a floating point literal so presumably also floating point.
promag
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
ACK 1891245, added cast to double and also braces.
|
ACK 1891245 I tested this with Clang 10 and the warning is gone. Nit: the cast to |
|
ACK 1891245 -- patch looks correct |
|
Would be nice to add (functional) tests for this, since it is uncovered right now. |
- doc: fix git add argument bitcoin#18513 - build: Fix libevent linking for bench_bitcoin binary bitcoin#18397 - script: fix SCRIPT_ERR_SIG_PUSHONLY error string bitcoin#18412 - doc: Comment fix merkle.cpp bitcoin#18379 - log: Fix UB with bench on genesis block bitcoin#15283 - test: Fix mining to an invalid target + ensure that a new block has the correct hash internally bitcoin#18350 - Fix missing header in sync.h bitcoin#18357 - refactor: Fix implicit value conversion in formatPingTime bitcoin#18260 - Fix .gitignore policy in build_msvc directory bitcoin#18108 - build: Fix behavior when ALLOW_HOST_PACKAGES unset bitcoin#18051 - test: Fix p2p_invalid_messages failing in Python 3.8 because of warning bitcoin#17931 - qa: Fix double-negative arg test bitcoin#17893 - build: Fix configure report about qr bitcoin#17547 - log: Fix log message for -par=1 bitcoin#17325 - bench: Fix negative values and zero for -evals flag bitcoin#17267 - depends: fix boost mac cross build with clang 9+ bitcoin#17231 - doc: Fix broken bitcoin-cli examples bitcoin#17119 - doc: fix Makefile target in benchmarking.md bitcoin#17081 - contrib: fix minor typos in makeseeds.py bitcoin#17042 - test: Fix Python Docstring to include all Args. bitcoin#17030 - doc: Fix some misspellings bitcoin#17351 - doc: Doxygen-friendly script/descriptor.h comments bitcoin#16947 - doc: Fix doxygen errors bitcoin#17945 - doc: Doxygen-friendly CuckooCache comments bitcoin#16986 - doc: Add to Doxygen documentation guidelines bitcoin#17873 - depends: Consistent use of package variable bitcoin#17928 - init: Replace URL_WEBSITE with PACKAGE_URL bitcoin#18503 - doc: Add missed copyright headers bitcoin#17691
Summary: ``` This changes CNodeStats to handle ping timestamps as their original incoming usec int64_t values until the time they need to be displayed. ``` Backport of core [[bitcoin/bitcoin#18260 | PR18260]]. This fix an implicit conversion warning from Clang. Test Plan: With clang as a compiler: ninja check Check the warning is gone. ./src/qt/bitcoin-qt Under the debug console => peer table, check the ping times look correct. Reviewers: #bitcoin_abc, deadalnix Reviewed By: #bitcoin_abc, deadalnix Differential Revision: https://reviews.bitcoinabc.org/D6277
Alternative to #18252, see motivation there.
This changes
CNodeStatsto handle ping timestamps as their original incoming usecint64_tvalues until the time they need to be displayed.