Alexey Serbin has posted comments on this change. ( http://gerrit.cloudera.org:8080/15156 )
Change subject: [clock] simplify system time source ...................................................................... Patch Set 3: (4 comments) http://gerrit.cloudera.org:8080/#/c/15156/3/src/kudu/clock/system_ntp.cc File src/kudu/clock/system_ntp.cc: http://gerrit.cloudera.org:8080/#/c/15156/3/src/kudu/clock/system_ntp.cc@74 PS3, Line 74: > Both CallAdjTime and NtpGetTime are only used once, so I think it'd be more Done http://gerrit.cloudera.org:8080/#/c/15156/3/src/kudu/clock/system_ntp.cc@81 PS3, Line 81: Status NtpGetTime(ntptimeval* tv) { > Can you rework this a bit to avoid the call to ntp_gettime() if FLAGS_injec Done http://gerrit.cloudera.org:8080/#/c/15156/3/src/kudu/clock/system_ntp.cc@82 PS3, Line 82: int rc = ntp_gettime(tv); > Regarding return values, the manpage says: Good idea. Done. http://gerrit.cloudera.org:8080/#/c/15156/3/src/kudu/clock/system_ntp.cc@133 PS3, Line 133: tv.time.tv_sec * 1000000 > Can this overflow? Do we need to explicitly cast one operand to uint64_t? Good catch. Added static_cast. -- To view, visit http://gerrit.cloudera.org:8080/15156 To unsubscribe, visit http://gerrit.cloudera.org:8080/settings Gerrit-Project: kudu Gerrit-Branch: master Gerrit-MessageType: comment Gerrit-Change-Id: Ib197effb63e8f5e37fd828c88e705112bb7047ce Gerrit-Change-Number: 15156 Gerrit-PatchSet: 3 Gerrit-Owner: Alexey Serbin <[email protected]> Gerrit-Reviewer: Adar Dembo <[email protected]> Gerrit-Reviewer: Alexey Serbin <[email protected]> Gerrit-Reviewer: Kudu Jenkins (120) Gerrit-Reviewer: Volodymyr Verovkin <[email protected]> Gerrit-Comment-Date: Tue, 04 Feb 2020 23:58:21 +0000 Gerrit-HasComments: Yes
