gui: fix compiler warning in formatPingTime() #18252
Conversation
With Clang 10.0.0:
```
qt/guiutil.cpp:778:26: warning: implicit conversion from 'std::__1::numeric_limits<long>::type'
(aka 'long') to 'double' changes value from 9223372036854775807 to 9223372036854775808
[-Wimplicit-int-float-conversion]
return (dPingTime == std::numeric_limits<int64_t>::max()/1e6 || dPingTime == 0) ? QObject::...
^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
```
When assigning to `dPingTime` a similar typecast is used:
```
src/net.cpp:558: stats.dPingTime = (((double)nPingUsecTime) / 1e6);
```
so it should be ok to change 9223372036854775807 to 9223372036854775808.
|
Here's a proposed alternative - representing these values as int64_t internally, and only converting to double at the time of display. Motivated by a general distrust of floating point and a desire to minimize the points at which it's used: |
maflcko
changed the title
|
Concept ACK on fixing but I prefer @Empact's approach :) |
|
From a quick look and comparison I also prefer @Empact approach. |
|
Opened the alternative as #18260 |
|
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers. ConflictsReviewers, this pull request conflicts with the following ones:
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first. |
|
Given the comments here, I'm going to close this in favour of #18260. |
|
Excellent! Yes, #18260 clearly supersedes this PR. |
1891245 refactor: Cast ping values to double before output (Ben Woosley) 7a810b1 refactor: Convert ping wait time from double to int64_t (Ben Woosley) e6fc63e refactor: Convert min ping time from double to int64_t (Ben Woosley) b054c46 refactor: Convert ping time from double to int64_t (Ben Woosley) Pull request description: Alternative to #18252, see motivation there. This changes `CNodeStats` to handle ping timestamps as their original incoming usec `int64_t` values until the time they need to be displayed. ACKs for top commit: vasild: ACK 1891245 practicalswift: ACK 1891245 -- patch looks correct promag: ACK 1891245, added cast to double and also braces. Tree-SHA512: 7cfcba941d9751b522b8c512c25da493338b444637bd0bb711b152d7d86b431ca0968956be3c844ee9dbfea25edab44a0de2afa44f2c9c0bf5b8df53eba66272
6 participants
With Clang 10.0.0:
When assigning to
dPingTimea similar typecast is used:so it should be ok to change 9223372036854775807 to 9223372036854775808.