Topic/delia 69767 #225

jthomp007c · 2025-12-17T14:40:16Z

I have been working on a ticket LLAMA-17349 for a while, trying to figure out how a file descriptor used by one thread in controlMgr is being closed by another controlMgr thread.
Finally I found that the call to cat in cacheEventToFile line "fs = popen ("cat /tmp/t2_caching_file | wc -l", "r");" was closing my thread's fd.
Why this happens I can't really explain, but I have proven that replacing the popen() call with standard C to count line numbers makes the problem not happen anymore.

Reason for change: Somehow the execve calling cat in the popen() function is closing a file descriptor used by another thread Test Procedure: Standard NSM chamber testing Risks: Low Priority: P1 Signed-off-by: Jason Thomson <jason_thomson@comcast.com>

rdkcmf-jenkins · 2025-12-17T14:50:19Z

source/commonlib/telemetry_busmessage_sender.c

+
+    fs = fopen(T2_CACHE_FILE, "r");
+    if (fs != NULL) {
+        while ((ch = fgetc(fs)) != EOF) {


Coverity issue no longer present as of: undefined

Show issue

Coverity Issue - Truncated stdio return value

Assigning the return value of "fgetc" to char "ch" truncates its value.

Medium Impact, CWE-197
CHAR_IO

Copilot

Pull request overview

This PR addresses a critical file descriptor management issue (LLAMA-17349) where popen() was inadvertently closing file descriptors used by other threads in controlMgr. The fix replaces the shell-based line counting (cat | wc -l) with native C file operations, and improves timestamp precision in debug logging.

Key Changes:

Replaced popen-based line counting with fgetc-based implementation to avoid fd interference
Enhanced EVENT_DEBUG logging with millisecond-precision timestamps using clock_gettime
Switched from localtime to thread-safe localtime_r for time conversion

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-17T16:00:24Z

source/commonlib/telemetry_busmessage_sender.c

+    fs = fopen(T2_CACHE_FILE, "r");
+    if (fs != NULL) {
+        while ((ch = fgetc(fs)) != EOF) {
+            if (ch == '\n') {
+                count++;
+            }
+        }
+
+        //If the file is not empty and does not contain a newline, call it one line
+        if (count == 0 && ftell(fs) > 0) {
+            count++;
+        }
+        fclose(fs);
+        fs = NULL;
    }


The static global variable count (declared at line 58) is never reset before counting lines, causing it to accumulate across multiple invocations of this function. Since cacheEventToFile is called as a thread function (line 730), each new thread will continue incrementing from the previous total, eventually causing all caching to be incorrectly disabled when count exceeds MAX_EVENT_CACHE (200). The count should be reset to 0 at the start of this line counting block or made a local variable.

Copilot · 2025-12-17T16:00:24Z

source/commonlib/telemetry_busmessage_sender.c

    FILE *fs = NULL;
-    char path[100];
-    pthread_detach(pthread_self());
+    int ch;


The removal of pthread_detach(pthread_self()) may cause resource leaks. When threads are created via pthread_create at line 730 without the detached attribute, they need to either be joined or explicitly detached to release their resources when they terminate. Since this function is meant to run asynchronously and the calling code doesn't join the thread, the thread should either be created with PTHREAD_CREATE_DETACHED attribute or call pthread_detach(pthread_self()) as was done previously.

Could you please retain the thread attribute to detached.

source/commonlib/telemetry_busmessage_sender.c

Copilot · 2025-12-17T16:00:25Z

source/commonlib/telemetry_busmessage_sender.c

+        while ((ch = fgetc(fs)) != EOF) {
+            if (ch == '\n') {
+                count++;
+            }
+        }
+
+        //If the file is not empty and does not contain a newline, call it one line
+        if (count == 0 && ftell(fs) > 0) {


After reading through the file with fgetc until EOF, ftell may not accurately reflect whether the file had content, especially if fgetc consumed all characters. The file position after EOF doesn't reliably indicate the original file size. Consider using fseek to get the file size before reading, or check if any characters were read during the loop instead of relying on ftell after EOF.

Suggested change

while ((ch = fgetc(fs)) != EOF) {

if (ch == '\n') {

count++;

}

}

//If the file is not empty and does not contain a newline, call it one line

if (count == 0 && ftell(fs) > 0) {

long file_size = 0;

/* Determine file size before reading, then rewind to start */

if (fseek(fs, 0, SEEK_END) == 0) {

long pos = ftell(fs);

if (pos > 0) {

file_size = pos;

}

rewind(fs);

}

while ((ch = fgetc(fs)) != EOF) {

if (ch == '\n') {

count++;

}

}

/* If the file is not empty and does not contain a newline, call it one line */

if (count == 0 && file_size > 0) {

Copilot · 2025-12-17T16:00:25Z

source/commonlib/telemetry_busmessage_sender.c

+        strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);
+        snprintf(timeBuffer + strlen(timeBuffer), sizeof(timeBuffer) - strlen(timeBuffer), ".%03ld", msecs);


The snprintf call builds on top of the existing string in timeBuffer by using strlen(timeBuffer) as the offset. While this works, it's more fragile than necessary. Consider using a single snprintf call with a combined format string, or at minimum verify the return value from strftime to ensure it succeeded before appending.

Suggested change

strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);

snprintf(timeBuffer + strlen(timeBuffer), sizeof(timeBuffer) - strlen(timeBuffer), ".%03ld", msecs);

size_t len = strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);

if (len > 0 && len < sizeof(timeBuffer)) {

int ret = snprintf(timeBuffer + len, sizeof(timeBuffer) - len, ".%03ld", msecs);

if (ret < 0) {

timeBuffer[len] = '\0';

}

} else {

timeBuffer[0] = '\0';

}

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

shibu-kv

Could you please apply the formatting as well that got flagged in L2 workflows. Astyle code formatting commands would be -

            find . -name '*.c' -o -name '*.h' | xargs astyle --options=.astylerc
            find . -name '*.orig' -type f -delete

Rest of the changes looks good.

shibu-kv · 2025-12-17T21:47:06Z

source/commonlib/telemetry_busmessage_sender.c

    FILE *fs = NULL;
-    char path[100];
-    pthread_detach(pthread_self());
+    int ch;


Could you please retain the thread attribute to detached.

jthomp007c added 2 commits December 16, 2025 17:12

Fixing var errors

297b0a9

jthomp007c requested a review from a team as a code owner December 17, 2025 14:40

rdkcmf-jenkins reviewed Dec 17, 2025

View reviewed changes

Fixing coverity issue

597a786

shibu-kv requested a review from Copilot December 17, 2025 15:52

Copilot started reviewing on behalf of shibu-kv December 17, 2025 15:53 View session

Copilot AI reviewed Dec 17, 2025

View reviewed changes

Apply suggestion from @Copilot

cccb5a6

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>

shibu-kv requested changes Dec 17, 2025

View reviewed changes

dharshini555 added 2 commits December 24, 2025 10:54

Update telemetry_busmessage_sender.c

f2712f9

Resolve formatting errors in telemetry_busemessage_sender.c

7b8118a

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Topic/delia 69767 #225

Topic/delia 69767 #225

jthomp007c commented Dec 17, 2025

Uh oh!

rdkcmf-jenkins Dec 17, 2025 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

shibu-kv Dec 17, 2025

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

Copilot AI Dec 17, 2025

Uh oh!

shibu-kv left a comment •

edited

Loading

Uh oh!

shibu-kv Dec 17, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

-        while ((ch = fgetc(fs)) != EOF) {
-            if (ch == '\n') {
-                count++;
-            }
-        }
-        //If the file is not empty and does not contain a newline, call it one line
-        if (count == 0 && ftell(fs) > 0) {
+        long file_size = 0;
+        /* Determine file size before reading, then rewind to start */
+        if (fseek(fs, 0, SEEK_END) == 0) {
+            long pos = ftell(fs);
+            if (pos > 0) {
+                file_size = pos;
+            }
+            rewind(fs);
+        }
+        while ((ch = fgetc(fs)) != EOF) {
+            if (ch == '\n') {
+                count++;
+            }
+        }
+        /* If the file is not empty and does not contain a newline, call it one line */
+        if (count == 0 && file_size > 0) {

		strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);
		snprintf(timeBuffer + strlen(timeBuffer), sizeof(timeBuffer) - strlen(timeBuffer), ".%03ld", msecs);

-        strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);
-        snprintf(timeBuffer + strlen(timeBuffer), sizeof(timeBuffer) - strlen(timeBuffer), ".%03ld", msecs);
+        size_t len = strftime(timeBuffer, sizeof(timeBuffer), "%Y-%m-%d %H:%M:%S", &timeinfo);
+        if (len > 0 && len < sizeof(timeBuffer)) {
+            int ret = snprintf(timeBuffer + len, sizeof(timeBuffer) - len, ".%03ld", msecs);
+            if (ret < 0) {
+                timeBuffer[len] = '\0';
+            }
+        } else {
+            timeBuffer[0] = '\0';
+        }

Topic/delia 69767 #225

Are you sure you want to change the base?

Topic/delia 69767 #225

Conversation

jthomp007c commented Dec 17, 2025

Uh oh!

rdkcmf-jenkins Dec 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Coverity Issue - Truncated stdio return value

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

shibu-kv Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

shibu-kv left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

shibu-kv Dec 17, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

rdkcmf-jenkins Dec 17, 2025 •

edited

Loading

shibu-kv left a comment •

edited

Loading