refactor: Fix implicit value conversion in formatPingTime #18260

pull Empact wants to merge 4 commits into bitcoin:master from Empact:2020-03-ping-time changing 7 files +24 −21

Empact commented at 6:53 PM on March 4, 2020: member

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.
refactor: Convert ping time from double to int64_t b054c46977
refactor: Convert min ping time from double to int64_t e6fc63ec7e
refactor: Convert ping wait time from double to int64_t 7a810b1d7a
practicalswift commented at 7:01 PM on March 4, 2020: contributor

Concept ACK
DrahtBot commented at 8:08 PM on March 4, 2020: member



The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.



Conflicts

No conflicts as of last run.

in src/rpc/net.cpp:169 in 7a810b1d7a outdated

 170 | -        if (stats.dMinPing < static_cast<double>(std::numeric_limits<int64_t>::max())/1e6)
 171 | -            obj.pushKV("minping", stats.dMinPing);
 172 | -        if (stats.dPingWait > 0.0)
 173 | -            obj.pushKV("pingwait", stats.dPingWait);
 174 | +        if (stats.m_ping_usec > 0)
 175 | +            obj.pushKV("pingtime", stats.m_ping_usec / 1e6);

promag commented at 8:46 PM on March 4, 2020:

Should cast to double first?

Empact commented at 9:46 AM on March 5, 2020:

Done

vasild commented at 5:23 PM on March 5, 2020:

Assuming x is int64_t, then all of these are equivalent:

x / 1e6;
(double)x / 1e6;
((double)x) / 1e6;
x / 1000000.0;

promag commented at 6:07 PM on March 5, 2020:

Indeed, I tend to prefer the explicit numerator cast though.

DrahtBot added the label GUI on Mar 4, 2020
DrahtBot added the label P2P on Mar 4, 2020
DrahtBot added the label Refactoring on Mar 4, 2020
DrahtBot added the label RPC/REST/ZMQ on Mar 4, 2020
hebasto commented at 9:31 PM on March 4, 2020: member

Concept ACK.
fanquake removed the label GUI on Mar 4, 2020
fanquake removed the label P2P on Mar 4, 2020
fanquake removed the label RPC/REST/ZMQ on Mar 4, 2020
fanquake commented at 12:11 AM on March 5, 2020: member

cc @vasild

refactor: Cast ping values to double before output

Note the divisor is a floating point literal so presumably
also floating point.

1891245e73

promag commented at 4:51 PM on March 5, 2020: member

ACK 1891245e7318bf625bbf67aab08a79fc3e87b61d, added cast to double and also braces.
vasild commented at 5:39 PM on March 5, 2020: member

ACK 1891245

I tested this with Clang 10 and the warning is gone.

Nit: the cast to double added in 1891245 is not necessary (I am ok with it).
practicalswift commented at 6:37 PM on March 5, 2020: contributor

ACK 1891245e7318bf625bbf67aab08a79fc3e87b61d -- patch looks correct
MarcoFalke added the label RPC/REST/ZMQ on Mar 5, 2020
MarcoFalke merged this on Mar 5, 2020
MarcoFalke closed this on Mar 5, 2020
MarcoFalke commented at 6:52 PM on March 5, 2020: member

Would be nice to add (functional) tests for this, since it is uncovered right now.
Empact deleted the branch on Mar 6, 2020
MarkLTZ referenced this in commit 8823f65197 on Apr 7, 2020
MarkLTZ referenced this in commit 0a1571d657 on Apr 7, 2020
deadalnix referenced this in commit e61415e7b5 on May 28, 2020
kittywhiskers referenced this in commit 5bb90a91f1 on Aug 5, 2021
kittywhiskers referenced this in commit dd1c7a650e on Aug 5, 2021
kittywhiskers referenced this in commit cdc8e85494 on Aug 9, 2021
kittywhiskers referenced this in commit 86df1b2944 on Aug 12, 2021
kittywhiskers referenced this in commit f64f322e18 on Aug 22, 2021
kittywhiskers referenced this in commit f54cfc10fd on Aug 30, 2021
kittywhiskers referenced this in commit 8deafd17d5 on Nov 1, 2021
kittywhiskers referenced this in commit ab26151abe on Nov 1, 2021
kittywhiskers referenced this in commit d949c90bc0 on Nov 3, 2021
pravblockc referenced this in commit 9b119df98e on Nov 18, 2021
DrahtBot locked this on Feb 15, 2022

Contributors

Labels

Refactoring RPC/REST/ZMQ

Linked (view graph)

#18252 gui: fix compiler warning in formatPingTime()

refactor: Fix implicit value conversion in formatPingTime #18260

Conflicts