RDKEMW-16851: Fix for skipfrequency issue in top markers#332
Open
yogeswaransky wants to merge 9 commits intosupport/1.8from
Open
RDKEMW-16851: Fix for skipfrequency issue in top markers#332yogeswaransky wants to merge 9 commits intosupport/1.8from
yogeswaransky wants to merge 9 commits intosupport/1.8from
Conversation
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes stale Top Marker values when skipFreq causes marker processing to be skipped, by relocating/resetting per-interval marker values in the Top Marker processing flow.
Changes:
- Removed
cpuValue/memValuecleanup fromgetProcUsage(). - Added cleanup of
cpuValue/memValueinprocessTopPattern()before the skip-frequency check.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
source/dcautil/dcaproc.c |
Removes per-call freeing of TopMarker CPU/memory strings from getProcUsage(). |
source/dcautil/dca.c |
Clears previous Top Marker CPU/memory values before applying skip-frequency logic. |
Comments suppressed due to low confidence (1)
source/dcautil/dcaproc.c:235
getProcUsage()now overwritesmarker->cpuValue/marker->memValuewithstrdup()results without freeing any existing values. This changes the function’s ownership/cleanup contract and will leak memory if the sameTopMarkeris passed repeatedly without the caller clearing these fields first. Consider restoring the defensive free/nulling insidegetProcUsage()(it’s safe even if callers already cleared), or explicitly document/enforce the expectation that the caller must reset these fields before calling.
if(0 != getProcInfo(&pInfo, filename))
{
T2Debug("Process info - CPU: %s, Memory: %s \n", pInfo.cpuUse, pInfo.memUse);
marker->cpuValue = strdup(pInfo.cpuUse);
marker->memValue = strdup(pInfo.memUse);
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Fixes skip-frequency behavior for “top” markers (CPU/memory) in the DCA utility flow, and expands CI coverage to additional maintenance branches.
Changes:
- Increase
BUF_LENused byprocMemCpuInfofields to avoid truncation. - Move/reset handling of
TopMarkerCPU/memory values and adjusttopparsing PID fallback flow. - Trigger L1/L2 GitHub Actions workflows for PRs targeting
support/1.8in addition todevelop.
Reviewed changes
Copilot reviewed 5 out of 5 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
source/dcautil/dcautil.h |
Expands BUF_LEN constant used for process/CPU/mem string storage. |
source/dcautil/dcaproc.c |
Removes per-call freeing of marker CPU/mem strings; tweaks PID fallback braces in getCPUInfo(). |
source/dcautil/dca.c |
Adds freeing/reset of TopMarker CPU/mem values before skip-frequency logic in the first pass. |
.github/workflows/L2-tests.yml |
Runs L2 workflow for PRs into develop and support/1.8. |
.github/workflows/L1-tests.yml |
Runs L1 workflow for PRs into develop and support/1.8. |
Comments suppressed due to low confidence (1)
source/dcautil/dcaproc.c:236
getProcUsage()no longer frees existingmarker->cpuValue/marker->memValue, but it still overwrites them withstrdup(). Unless every caller guarantees these pointers are NULL before calling (which is not true for all markers inprocessTopPattern()due to the earlybreak), this will leak memory each time the marker is processed. Either restore the free+NULL logic here before assignment, or switch to updating through a helper that replaces/free+sets the value safely.
pInfo.total_instance = index;
pInfo.pid = pid;
if(0 != getProcInfo(&pInfo, filename))
{
T2Debug("Process info - CPU: %s, Memory: %s \n", pInfo.cpuUse, pInfo.memUse);
marker->cpuValue = strdup(pInfo.cpuUse);
marker->memValue = strdup(pInfo.memUse);
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
Signed-off-by: Yogeswaran K <yogeswaransky@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.