RDKEMW-10467: Fix Invalid time values caused by drift #212
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR fixes invalid time values in telemetry report generation caused by system clock drift by changing timing measurements from CLOCK_REALTIME to CLOCK_MONOTONIC.
- Changed all elapsed time measurements from
CLOCK_REALTIMEtoCLOCK_MONOTONICto prevent clock drift from affecting duration calculations - Improved log messages in
profilexconf.cfor better clarity and consistency
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| source/bulkdata/profilexconf.c | Updated three clock_gettime calls to use CLOCK_MONOTONIC for measuring start time, processing time, and elapsed time; improved log messages for execution count and processing time |
| source/bulkdata/profile.c | Updated two clock_gettime calls to use CLOCK_MONOTONIC for measuring start and end times of report collection and generation |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Code changes looks good to me. |
Coverity Issue - Data race conditionAccessing "profile->grepSeekProfile->execCounter" without holding lock "plMutex". Elsewhere, "_GrepSeekProfile.execCounter" is written to with "plMutex" held 2 out of 2 times. Medium Impact, CWE-366 Issue locationThis issue was discovered outside the diff for this Pull Request. You can find it at: |
| @@ -451,7 +451,7 @@ static void* CollectAndReportXconf(void* data) | |||
| T2Warning("Failed to save grep config to file for profile: %s\n", profile->name); | |||
| } | |||
There was a problem hiding this comment.
Coverity issue no longer present as of: undefined
Show issue
Coverity Issue - Unchecked return value
Calling "clock_gettime" without checking return value (as is done elsewhere 1 out of 1 times).
Medium Impact, CWE-252
CHECKED_RETURN
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| clockReturn |= clock_gettime(CLOCK_MONOTONIC, &endTime); | ||
| if (clockReturn) | ||
| { | ||
| T2Warning("Error in Fetching the time Elapsed"); |
There was a problem hiding this comment.
The error message "Error in Fetching the time Elapsed" is grammatically incorrect and unclear. Consider revising to "Error fetching elapsed time" or "Failed to get time from clock_gettime()" to be more concise and accurate.
| T2Warning("Error in Fetching the time Elapsed"); | |
| T2Warning("Failed to get time from clock_gettime()"); |
…ONIC Reason for Change: Changing CLOCK_REALTIME to CLOCK_MONOTONIC to avoid clock drifts impact in the calculations Test Procedure: When Telemetry report generation is in progress change the time. The processing time should have a valid value Risks: Low Priority: P1
shibu-kv
force-pushed
the
topic/RDKEMW-10467
branch
from
9a7e5ba to
d048d01
Compare
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Reason for Change: Changing CLOCK_REALTIME to CLOCK_MONOTONIC to avoid clock drifts impact in the calculations
Test Procedure: When Telemetry report generation is in progress change the time. The processing time should have a valid value
Risks: Low
Priority: P1