Feature/rdk 60236 dynamic#189
Open
Abhinavpv28 wants to merge 80 commits intodevelopfrom
Open
Conversation
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <175728472+Copilot@users.noreply.github.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR extends the remote debugger “issue type” handling to preserve an issue-type suffix (everything after the first _) across dynamic flows, caching, and final upload naming (so generated artifacts can retain unique identifiers).
Changes:
- Add
suffixfields todata_bufandcacheData, propagate suffix through cache, and incorporate it into upload file naming. - Introduce
split_issue_type()utility and update issue-type processing to split base issue type vs. suffix. - Update unit tests and ignore Autotools build artifacts via a new
.gitignore.
Reviewed changes
Copilot reviewed 10 out of 12 changed files in this pull request and generated 10 comments.
Show a summary per file
| File | Description |
|---|---|
| src/rrdCommon.h | Adds suffix fields to shared structs (data_buf, cacheData). |
| src/rrdEventProcess.c | Splits issue type into base+suffix and stores suffix in data_buf. |
| src/rrdJsonParser.h | Exposes split_issue_type() declaration. |
| src/rrdJsonParser.c | Implements split_issue_type() and uses suffix when building upload names. |
| src/rrdRunCmdThread.h | Updates createCache() signature and introduces an append_item() prototype with suffix. |
| src/rrdRunCmdThread.c | Adds cache suffix persistence and frees suffix in cache cleanup. |
| src/rrdDynamic.c | Passes suffix into cache append calls for dynamic flows. |
| src/rrdIarmEvents.c | Restores cached suffix into queued data_buf for later upload naming. |
| src/rrdInterface.c | Initializes/frees data_buf->suffix in init/dealloc helpers. |
| src/unittest/rrdUnitTestRunner.cpp | Updates createCache() test calls and adds suffix assertions. |
| src/unittest/UTJson/device.properties | Unit-test device properties file (content not shown in diff). |
| .gitignore | Ignores Autotools-generated build outputs. |
Comments suppressed due to low confidence (1)
src/unittest/rrdUnitTestRunner.cpp:1379
createCache(..., strdup("mysuffix"))leaks memory:createCacheduplicatessuffixinternally (viastrdup), and the temporarystrdup("mysuffix")passed as the argument is never freed. Additionally,result->suffix(allocated insidecreateCache) is never freed beforefree(result), so the test will also leak that allocation and may fail under valgrind. Prefer passing a string literal/stack buffer as the input suffix and freeingresult->suffix(or using the cache node cleanup helper) during teardown.
char *issueTypeData = strdup("ValidIssueTypeData");
cacheData *result = createCache(pkgData, issueTypeData, strdup("mysuffix"));
ASSERT_STREQ(result->suffix, "mysuffix");
ASSERT_NE(result, nullptr);
ASSERT_STREQ(result->mdata, "ValidPkgData");
ASSERT_STREQ(result->issueString, "ValidIssueTypeData");
ASSERT_EQ(result->next, nullptr);
ASSERT_EQ(result->prev, nullptr);
free(pkgData);
free(issueTypeData);
free(result);
}
Comment on lines
+247
to
+252
| append_item(strdup(msgDataString), strdup(appendData), rbuf->suffix); | ||
| RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Setting Parameters Success and Cache Updated ...%s IssueStr:%s Length:%d\n", __FUNCTION__, __LINE__, msgDataString, appendData, strlen(appendData)); | ||
| } | ||
| else | ||
| { | ||
| append_item(strdup(msgDataString), strdup((char *)rbuf->mdata)); | ||
| append_item(strdup(msgDataString), strdup((char *)rbuf->mdata), rbuf->suffix); |
Comment on lines
232
to
+239
| if(rrdCachetmpnode != NULL) | ||
| { | ||
| free(rrdCachetmpnode->mdata); | ||
| free(rrdCachetmpnode->issueString); | ||
| if (rrdCachetmpnode->suffix) { | ||
| free(rrdCachetmpnode->suffix); | ||
| rrdCachetmpnode->suffix = NULL; | ||
| } |
Comment on lines
+1350
to
1354
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_EQ(result->mdata, nullptr); |
Comment on lines
+1402
to
1406
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_EQ(result->mdata, nullptr); |
Comment on lines
286
to
+302
| void RRD_data_buff_deAlloc(data_buf *sbuf) | ||
| { | ||
| if (sbuf) | ||
| { | ||
| if (sbuf->mdata) | ||
| { | ||
| free(sbuf->mdata); | ||
| } | ||
|
|
||
| if (sbuf->jsonPath) | ||
| { | ||
| free(sbuf->jsonPath); | ||
| } | ||
| if (sbuf->suffix) | ||
| { | ||
| free(sbuf->suffix); | ||
| } |
Comment on lines
118
to
+120
| RDK_LOG(RDK_LOG_DEBUG,LOG_REMDEBUG,"[%s:%d]: RRD Mutex Lock...%d\n",__FUNCTION__,__LINE__,i); | ||
|
|
||
| cacheData *tmp = createCache(pkgData,issueTypeData); | ||
| cacheData *tmp = createCache(pkgData, issueTypeData, suffix); |
Comment on lines
+54
to
57
| cacheData* createCache(char *pkgData, char *issueTypeData, char *suffix); | ||
| void append_item(char *pkgData, char *issueTypeData, char *suffix); | ||
| void print_items(cacheData *node); | ||
| void append_item(char *pkgData, char *issueTypeData); |
Comment on lines
+1367
to
1371
| cacheData *result = createCache(pkgData, issueTypeData, strdup("mysuffix")); | ||
| ASSERT_STREQ(result->suffix, "mysuffix"); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_STREQ(result->mdata, "ValidPkgData"); |
Comment on lines
+1385
to
1389
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_STREQ(result->mdata, "ValidPkgData"); |
| RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Cache.issueString=%s Cache.issueString.Len=%d\n", __FUNCTION__, __LINE__, cache->issueString, strlen(cache->issueString)); | ||
| strncpy((char *)sendbuf->mdata, cache->issueString, recPkgNamelen); | ||
| if (cache->suffix) { | ||
| sendbuf->suffix = strdup(cache->suffix); |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 7 comments.
Comments suppressed due to low confidence (1)
src/unittest/rrdUnitTestRunner.cpp:1378
- This test leaks memory related to
suffix: it passesstrdup("mysuffix")intocreateCache(), butcreateCache()duplicates the suffix again and the test never frees either the argument string orresult->suffix(it onlyfree(result)). Avoid double-allocation (pass a string literal / stack buffer), and ensure the allocatedresult->suffixis freed (e.g., viafreecacheDataCacheNode(&result)or by freeing the fields explicitly). Also,ASSERT_STREQ(result->suffix, ...)should come afterASSERT_NE(result, nullptr).
cacheData *result = createCache(pkgData, issueTypeData, strdup("mysuffix"));
ASSERT_STREQ(result->suffix, "mysuffix");
ASSERT_NE(result, nullptr);
ASSERT_STREQ(result->mdata, "ValidPkgData");
ASSERT_STREQ(result->issueString, "ValidIssueTypeData");
ASSERT_EQ(result->next, nullptr);
ASSERT_EQ(result->prev, nullptr);
free(pkgData);
free(issueTypeData);
free(result);
Comment on lines
+341
to
+345
| if (cache->suffix) { | ||
| sendbuf->suffix = strdup(cache->suffix); | ||
| RDK_LOG(RDK_LOG_DEBUG, LOG_REMDEBUG, "[%s:%d]: Restored suffix from cache: %s\n", __FUNCTION__, __LINE__, cache->suffix); | ||
| } else { | ||
| sendbuf->suffix = NULL; |
Comment on lines
+55
to
57
| void append_item(char *pkgData, char *issueTypeData, char *suffix); | ||
| void print_items(cacheData *node); | ||
| void append_item(char *pkgData, char *issueTypeData); |
Comment on lines
+1350
to
1354
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_EQ(result->mdata, nullptr); |
Comment on lines
+1385
to
1389
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_STREQ(result->mdata, "ValidPkgData"); |
Comment on lines
+1402
to
1406
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_EQ(result->mdata, nullptr); |
Comment on lines
+118
to
+123
| cacheData *tmp = createCache(pkgData, issueTypeData, suffix); | ||
| if(!tmp) | ||
| { | ||
| RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Memory Allocation Failed : Cannot Append Item to Cache\n", __FUNCTION__, __LINE__); | ||
| pthread_mutex_unlock(&rrdCacheMut); | ||
| return; | ||
| return; |
Comment on lines
83
to
90
| * @function createCache | ||
| * @brief Creates a cache node with the provided message data and issue string. | ||
| * @param char *pkgData - The package data to store in the cache node. | ||
| * @param char *issueTypeData - The issue type data to store in the cache node. | ||
| * @return cacheData* - Pointer to the created cache node, or NULL on failure. | ||
| */ | ||
| cacheData* createCache( char *pkgData, char *issueTypeData) | ||
| cacheData* createCache(char *pkgData, char *issueTypeData, char *suffix) | ||
| { |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 10 out of 12 changed files in this pull request and generated 5 comments.
Comments suppressed due to low confidence (1)
src/unittest/rrdUnitTestRunner.cpp:1378
createCachenow duplicatessuffixinternally (strdup), but this test passesstrdup("mysuffix")and never frees either the input allocation (pointer is lost) orresult->suffix. This will leak memory and can break valgrind-based test runs. Pass a non-allocated string (or reuse the same allocation without duplicating) and ensureresult->suffixis freed (or use the same cache cleanup helper used in production).
char *pkgData = strdup("ValidPkgData");
char *issueTypeData = strdup("ValidIssueTypeData");
cacheData *result = createCache(pkgData, issueTypeData, strdup("mysuffix"));
ASSERT_STREQ(result->suffix, "mysuffix");
ASSERT_NE(result, nullptr);
ASSERT_STREQ(result->mdata, "ValidPkgData");
ASSERT_STREQ(result->issueString, "ValidIssueTypeData");
ASSERT_EQ(result->next, nullptr);
ASSERT_EQ(result->prev, nullptr);
free(pkgData);
free(issueTypeData);
free(result);
| void append_item(char *pkgData, char *issueTypeData) | ||
| void append_item(char *pkgData, char *issueTypeData, char *suffix) | ||
| { | ||
| RDK_LOG(RDK_LOG_INFO,LOG_REMDEBUG,"[%s:%d]: Append Item with PkgData: %s and issue Type: %s to Cache \n",__FUNCTION__,__LINE__,pkgData,issueTypeData); |
Comment on lines
295
to
+302
| if (sbuf->jsonPath) | ||
| { | ||
| free(sbuf->jsonPath); | ||
| } | ||
| if (sbuf->suffix) | ||
| { | ||
| free(sbuf->suffix); | ||
| } |
| cacheData* createCache(char *pkgData, char *issueTypeData, char *suffix); | ||
| void append_item(char *pkgData, char *issueTypeData, char *suffix); | ||
| void print_items(cacheData *node); | ||
| void append_item(char *pkgData, char *issueTypeData); |
Comment on lines
+1350
to
1354
| cacheData *result = createCache(pkgData, issueTypeData, NULL); | ||
| ASSERT_EQ(result->suffix, nullptr); | ||
|
|
||
| ASSERT_NE(result, nullptr); | ||
| ASSERT_EQ(result->mdata, nullptr); |
Comment on lines
+82
to
86
| char base[128] = {0}; | ||
| char local_suffix[128] = {0}; | ||
| split_issue_type(cmdMap[index], base, sizeof(base), local_suffix, sizeof(local_suffix)); | ||
| dataMsgLen = strlen(base) + 1; | ||
| RRD_data_buff_init(cmdBuff, EVENT_MSG, RRD_DEEPSLEEP_INVALID_DEFAULT); /* Setting Deafult Values*/ |
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 join this conversation on GitHub.
Already have an account?
Sign in to comment
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.