RDKEMW-23188 : Test PR - #212
Open
balav08 wants to merge 41 commits into
Open
Conversation
RDKECMF-213 Add CODEOWNERS file
Signed-off-by: apatel859 <amit_patel5@comcast.com>
RDKEMW-254: libsoup3 support
DELIA-67407: Code syncup main to develop
Rebase with Develop Branch
…p_set_friendlyname Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
RDKTV-35185: Add sync between ssdp_http_server_callback and gdial_ssdp_set_friendlyname
Reason for change: Fix issues identified within xcast Test Procedure: Risks: low Priority: P1 Signed-off-by:Hayden Gfeller <Hayden_Gfeller@comcast.com>
…ialserver into feature/RDKEMW-2033
Reason for change: Implemented setManufacturerName and setModelName APIs for DIAL Server name configuration maintained the additional data url to specific app Test Procedure: DIAL should work Risks: None Priority: P1 Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…4_Dial_Args_1 RDK-55044: Implement DIAL requirement to use on EU product
RDKEMW-2033 : Coverity
Reason for change: Added DISABLE_SECURITY_TOKEN Flag to disable the WPEFrameworkSecurity Token generation changes Test Procedure: please referred from the ticket Risks: Medium Signed-off-by: Thamim Razith <ThamimRazith_AbbasAli@comcast.com>
RDKEMW-2278: Removal of WPEFrameworkSecurity Agent Utility
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_Coverity RDKEMW-4129: Prepare native build environment
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_CoverityTest RDKEMW-4129: Test
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…alserver into topic/RDKEMW-4129_CoverityTest Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…129_CoverityTest RDKEMW-4129: Prepare native build environment for Coverity
Signed-off-by: apatel859 <amit_patel5@comcast.com>
RDKEMW-4129: Prepare native build environment for Coverity
RDKEMW-2854 : Fix the double free issue on call to onApplicationStateChanged api
Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
* RDKEMW-6891: Coverity errors fix for xdial * Update gdial.cpp Fixed review comments
* RDKEMW-9964: Removing onStopped GDial notification handling Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com> * RDKEMW-9964: Fixed coverity issues Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com> * RDKEMW-9964: Fixed coverity issues Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com> * RDKEMW-9964: Fixed coverity issues Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com> --------- Signed-off-by: yuvaramachandran_gurusamy <yuvaramachandran_gurusamy@comcast.com>
…s issues in xdial (#182) * RDKEMW-11024 - Using copilot identify and fix the static code analysis issues in xdial Reason for Change: Resolving the static code issues scanned by copilot Test Procedure: Compiled and Verified Risks: Low Priority: P1 version: minor Signed-off-by: smohap466 <srinibas_mohapatra@comcast.com> --------- Signed-off-by: smohap466 <srinibas_mohapatra@comcast.com> Co-authored-by: smohap466 <srinibas_mohapatra@comcast.com> Co-authored-by: dkumar798 <dinesh_kumar2@comcast.com>
* RDKEMW-12059: Fix Coverity identified issues
* RDKEMW-12555 : Fix coveirty workflow scan in xdialserver repo Reason for Change: Fix coveirty scan workflow failure in xdialserver repo Test Procedure: Verify coveirty workflow Risks: Low Priority: P1 version: minor Signed-off-by:AkshayKumar_Gampa AkshayKumar_Gampa@comcast.com * RDKEMW-12555 : Fix coveirty workflow scan in xdialserver repo Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
* RDKEMW-12059: Fix Coverity identified issues * Update gdial-plat-util.c * Update gdial-rest.c * Update gdial-plat-util.c * Update gdial-plat-util.c
* RDKEMW-16911 - L1 unit tests for xdialserver
…M_SLEEP_REQUEST_KEY not configured (#206) * RDKEMW-19912: Update gdial.cpp * RDKEMW-19912: Fix for L1 test failure Signed-off-by: Yuvaramachandran Gurusamy <yuvaramachandran_gurusamy@comcast.com>
* RDKEMW-6898: gdialserver not compatible with Libsoup3 library Reason for change: gdialserver not compatible with Libsoup3 library Updating gssdp to 1.6.3 provides libsoup3 compatibility . Test Procedure: Compiled and Verified Risks: Low Priority: P1 version: minor Signed-off-by: balaji velmurugan <balaji_velmurugan@comcast.com> * Update Makefile.am * Update configure.ac * Update configure.ac * Update Makefile.am * Update configure.ac * Update configure.ac --------- Signed-off-by: balaji velmurugan <balaji_velmurugan@comcast.com> Co-authored-by: balaji velmurugan <balaji_velmurugan@comcast.com>
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
|
|
||
| steps: | ||
| - name: Set up CMake | ||
| uses: jwlawson/actions-setup-cmake@v1.13 |
| echo "Trigger Source: ${{ inputs.caller_source }}" | ||
|
|
||
| - name: Set up CMake | ||
| uses: jwlawson/actions-setup-cmake@v1.13 |
Comment on lines
+11
to
+25
| name: Build xdialserver component in github rdkcentral | ||
| runs-on: ubuntu-latest | ||
| container: | ||
| image: ubuntu:22.04 | ||
|
|
||
| steps: | ||
| - name: Checkout code | ||
| uses: actions/checkout@v3 | ||
|
|
||
| - name: native build | ||
| run: | | ||
| sh -x build_dependencies.sh | ||
| sh -x cov_build.sh | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.RDKCM_RDKE }} No newline at end of file |
Comment on lines
+41
to
+42
| g_print_with_timestamp("server_request_remove_callback tid=[%lx] msg=%p timeout source %d removed", | ||
| pthread_self(), msg, conn_context->read_timeout_source); |
| SoupMessage *msg = (SoupMessage*)user_data; | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| DialShieldConnectionContext * conn_context = (DialShieldConnectionContext *)g_hash_table_lookup(active_conns_, msg); | ||
| g_print_with_timestamp("soup_message_read_timeout_callback tid=[%lx] msg=%p", pthread_self(), msg); |
| static void server_request_read_callback (SoupServer *server, SoupServerMessage *msg, | ||
| gpointer data) { | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| g_print_with_timestamp("server_request_read_callback tid=[%lx] msg=%p", pthread_self(), msg); |
| static void server_request_aborted_callback (SoupServer *server, SoupServerMessage *msg, | ||
| gpointer data) { | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| g_print_with_timestamp("server_request_aborted_callback tid=[%lx] msg=%p", pthread_self(), msg); |
| SoupMessage *msg=(SoupMessage*)user_data; | ||
| GDIAL_LOGTRACE("Entering ..."); | ||
| assert(msg0==msg); | ||
| g_print_with_timestamp("soup_message_weak_ref_callback tid=[%lx] msg=%p", pthread_self(), msg); |
Comment on lines
+109
to
+110
| g_print_with_timestamp("server_request_started_callback tid=[%lx] msg=%p timeout source %d added with socket fd = %d", | ||
| pthread_self(), msg, read_timeout_source, g_socket_get_fd(conn_context->read_gsocket)); |
| uuid_unparse(random_uuid, uuid_data); | ||
| GDIAL_LOGINFO("generated uuid:[%s]", uuid_data); | ||
|
|
||
| fuuid = fopen(app_uuid_file_path, "w"); |
There was a problem hiding this comment.
Pull request overview
This PR introduces an L1 (unit-level) test harness for xdialserver, expands platform/service APIs to support manufacturer/model updates, and adds build/CI plumbing (autotools + GitHub Actions) including conditional libsoup 2.4/3.0 support and stubs for external dependencies.
Changes:
- Add L1 test runner (
run_L1Tests) with extensive unit tests across server/plat/utils plus mocks/stubs. - Extend service/platform plumbing (callbacks + request events) and SSDP handling for manufacturer/model + UUID-based local REST routing.
- Add build system support (autotools files, updated CMake, scripts) and CI workflows to build/run tests + coverage/valgrind.
Reviewed changes
Copilot reviewed 59 out of 60 changed files in this pull request and generated 12 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/README.md | Documents the L1 test harness layout and workflows. |
| tests/Makefile.am | Adds autotools test subdir build integration (ENABLE_L1TESTS). |
| tests/L1Tests/utils/test_gdialUtil.cpp | Adds unit tests for gdial-util helpers. |
| tests/L1Tests/tests/gdialserver_ut.cpp | Adds placeholder/template gtest file. |
| tests/L1Tests/test_main.cpp | Provides gtest/gmock main for run_L1Tests. |
| tests/L1Tests/stubs/gdial_cpp_test_stubs.hpp | Provides lightweight C++ stubs for gdial.cpp dependencies. |
| tests/L1Tests/server/test_gdialSsdp.cpp | Adds SSDP tests (dd.xml callback + setters). |
| tests/L1Tests/server/test_gdialShield.cpp | Adds tests for shielded SoupServer behavior. |
| tests/L1Tests/server/test_gdialService.cpp | Adds service/service-impl lifecycle + request-path tests. |
| tests/L1Tests/server/test_gdialServer.cpp | Adds tests for gdial-app object and helpers. |
| tests/L1Tests/server/test_gdialserver_ut.cpp | Adds unit tests around gdialserver_ut main loop using a fake service. |
| tests/L1Tests/server/test_gdialRest.cpp | Adds REST server tests (register/unregister, HTTP verbs, builder). |
| tests/L1Tests/plat/test_gdialPlatUtil.cpp | Adds tests for platform util + logging helpers. |
| tests/L1Tests/plat/test_gdialPlatDev.cpp | Adds tests for platform dev callbacks/power state helpers. |
| tests/L1Tests/plat/test_gdialPlatApp.cpp | Adds tests for gdial-plat-app API guards + async paths. |
| tests/L1Tests/plat/test_gdialPlat.cpp | Adds tests for GDialAppRegistry creation/origin logic/dispose. |
| tests/L1Tests/plat/test_gdialCpp.cpp | Adds tests that compile gdial.cpp into the TU with symbol remapping. |
| tests/L1Tests/plat/test_gdialAppCache.cpp | Adds tests for GDialAppStatusCache ID handling. |
| tests/L1Tests/plat/gdial_os_stubs.cpp | Adds OS-layer stubs used by L1 tests (below gdial-plat-app.c). |
| tests/L1Tests/mocks/gssdp_mock.c | Adds minimal GSSDP mocks for SSDP code paths. |
| tests/L1Tests/Makefile.am | Defines run_L1Tests sources, flags, and libsoup/gssdp conditionals. |
| stubs/securityagent/SecurityTokenUtil.h | Adds SecurityAgent token util stub header. |
| stubs/securityagent/SecurityTokenUtil.cpp | Adds SecurityAgent token util stub implementation. |
| stubs/iarm_stubs.cpp | Adds IARM bus stub library implementation. |
| server/plat/gdialappcache.hpp | Initializes observer pointer to avoid uninitialized use. |
| server/plat/gdialappcache.cpp | Uses moves for IDs, improves logging format, adds null checks. |
| server/plat/gdial.hpp | Adds manufacturer/model callback typedefs + registration APIs. |
| server/plat/gdial-plat-util.c | Adjusts thread-id formatting and adds static-analysis annotations. |
| server/plat/gdial-plat-dev.c | Removes manufacturer/model env getters. |
| server/plat/gdial-plat-app.c | Adds manufacturer/model callbacks + update APIs plumbing. |
| server/plat/gdial-os-app.h | Adds OS-layer APIs for manufacturer/model updates. |
| server/plat/gdial_app_registry.c | Adds persisted UUID-per-app and stores it in app_uri. |
| server/plat/CMakeLists.txt | Adds DISABLE_SECURITY_TOKEN option and adjusts link libraries. |
| server/libsoup/3.0/gdial-ssdp.c | Adds libsoup-3.0 compatible SSDP implementation. |
| server/libsoup/3.0/gdial-shield.c | Adds libsoup-3.0 compatible shield implementation. |
| server/libsoup/2.4/gdial-ssdp.c | Updates libsoup-2.4 SSDP with mutex + manufacturer/model overrides. |
| server/libsoup/2.4/gdial-shield.c | Adds libsoup-2.4 shield implementation file. |
| server/libsoup/2.4/gdial-rest.c | Extends local REST routing and additionalDataUrl generation using UUID URIs. |
| server/include/gdialserviceimpl.h | Extends request payload/events; initializes observer pointer. |
| server/include/gdialservicecommon.h | Removes onStopped() from notifier interface. |
| server/include/gdialservice.h | Adds setManufacturerName / setModelName to public service API. |
| server/include/gdial-plat-dev.h | Removes manufacturer/model getter declarations. |
| server/include/gdial-plat-app.h | Adds manufacturer/model callbacks and update APIs. |
| server/include/gdial-config.h | Renames include guard; removes unused error enum. |
| server/include/gdial_app_registry.h | Adds app_uri field and UUID size constant. |
| server/gdialservice.cpp | Wires manufacturer/model callbacks, libsoup3 adaptations, request handling updates. |
| server/gdialserver_ut.cpp | Updates notifier implementation for interface changes and uses std::move. |
| server/gdial-ssdp.h | Exposes manufacturer/model setter APIs. |
| server/gdial-rest.h | Updates function signatures and adds UUID registry lookup API. |
| server/CMakeLists.txt | Adds libsoup 2.4 vs 3.0 selection logic and correct source set. |
| Makefile.am | Adds autotools top-level SUBDIRS for tests. |
| Makefile | Passes DISABLE_SECURITY_TOKEN option through to CMake invocation. |
| cov_build.sh | Adds simple build script for CI. |
| configure.ac | Adds autotools configuration, deps, and --enable-l1tests. |
| build_dependencies.sh | Installs deps, builds external components, and builds stub shared libs. |
| .github/workflows/native_full_build.yml | Adds native-container build workflow. |
| .github/workflows/L1-tests.yml | Adds CI job to build/run L1 tests + valgrind + coverage. |
| .github/CODEOWNERS | Adds default code owners. |
Suppressed comments (3)
server/libsoup/2.4/gdial-ssdp.c:80
- ssdpServerEventSync is initialized with PTHREAD_MUTEX_INITIALIZER and then re-initialized with pthread_mutex_init() in gdial_ssdp_new(), which is undefined behavior for an already-initialized mutex.
server/libsoup/2.4/gdial-ssdp.c:264 - gdial_ssdp_destroy() removes a handler for "/dd.xml", but gdial_ssdp_new() registers the handler at "/<random_uuid>/dd.xml". This leaves the real handler installed and can lead to callbacks firing after teardown (use-after-free risk) and leaks.
server/libsoup/2.4/gdial-rest.c:1018 - gdial_rest_server_registered_apps_clear now expects a GDialRestServer*, but gdial_rest_server_dispose passes a GObject* (implicit conversion). This compiles with warnings and is easy to fix by passing GDIAL_REST_SERVER(object).
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+45
to
+55
| int GetSecurityToken(unsigned short maxLength, unsigned char buffer[]) | ||
| { | ||
| // get a localhost token | ||
| string payload = "http://localhost"; | ||
|
|
||
| size_t len = payload.length(); | ||
|
|
||
| if(!memcpy(buffer,payload.c_str(),len)) | ||
| return -1; | ||
| return 0; | ||
| } |
Comment on lines
+117
to
+122
| } | ||
| else | ||
| { | ||
| fgets(uuid_data, sizeof(uuid_data), fuuid); | ||
| fclose(fuuid); | ||
| } |
| static gchar *app_random_uuid = NULL; | ||
| static gchar *app_manufacturer_name = NULL; | ||
| static gchar *app_model_name = NULL; | ||
| static pthread_mutex_t ssdpServerEventSync = PTHREAD_MUTEX_INITIALIZER; |
Comment on lines
+250
to
+253
| app_random_uuid = g_strdup(random_uuid); | ||
| gchar *dail_ssdp_handler = g_strdup_printf("/%s/%s", random_uuid,"dd.xml"); | ||
| soup_server_add_handler(ssdp_http_server_, dail_ssdp_handler, ssdp_http_server_callback, NULL, NULL); | ||
| ssdp_client_ = ssdp_client; |
Comment on lines
+260
to
+264
| pthread_mutex_lock(&ssdpServerEventSync); | ||
| if (ssdp_http_server_) | ||
| { | ||
| soup_server_remove_handler(ssdp_http_server_, "/dd.xml"); | ||
| } |
Comment on lines
+38
to
+53
| static std::string BuildServerBaseUrl(SoupServer *srv) { | ||
| GSList *uris = soup_server_get_uris(srv); | ||
| if (!uris) return ""; | ||
| SoupURI *uri = static_cast<SoupURI *>(uris->data); | ||
| if (!uri) { | ||
| g_slist_free(uris); | ||
| return ""; | ||
| } | ||
|
|
||
| char *uri_str = soup_uri_to_string(uri, FALSE); | ||
| std::string base = uri_str ? uri_str : ""; | ||
| g_free(uri_str); | ||
| soup_uri_free(uri); | ||
| g_slist_free(uris); | ||
| return base; | ||
| } |
Comment on lines
+66
to
+72
| GSList *uris = soup_server_get_uris(server); | ||
| ASSERT_NE(uris, nullptr); | ||
| SoupURI *uri = (SoupURI *)uris->data; | ||
| guint port = soup_uri_get_port(uri); | ||
| base_url = std::string("http://127.0.0.1:") + std::to_string(port); | ||
| g_slist_free_full(uris, (GDestroyNotify)soup_uri_free); | ||
|
|
Comment on lines
+68
to
+80
| GSList *rest_uris = soup_server_get_uris(rest_server); | ||
| ASSERT_NE(rest_uris, nullptr); | ||
| SoupURI *rest_uri = (SoupURI *)rest_uris->data; | ||
| guint rest_port = soup_uri_get_port(rest_uri); | ||
| rest_base = std::string("http://127.0.0.1:") + std::to_string(rest_port) + "/" + rest_route_id; | ||
| g_slist_free_full(rest_uris, (GDestroyNotify)soup_uri_free); | ||
|
|
||
| GSList *local_uris = soup_server_get_uris(local_rest_server); | ||
| ASSERT_NE(local_uris, nullptr); | ||
| SoupURI *local_uri = (SoupURI *)local_uris->data; | ||
| guint local_port = soup_uri_get_port(local_uri); | ||
| local_base = std::string("http://127.0.0.1:") + std::to_string(local_port); | ||
| g_slist_free_full(local_uris, (GDestroyNotify)soup_uri_free); |
Comment on lines
+217
to
+226
| int gdial_os_application_register_applications(void *p) | ||
| { | ||
| if (s_registerapps_cb && p) { | ||
| const RegisterAppEntryList *app_config_list = static_cast<RegisterAppEntryList *>(p); | ||
| GList *g_app_list = build_app_registry_list(app_config_list); | ||
| s_registerapps_cb(g_app_list); | ||
| free_app_registry_list_nodes(g_app_list); | ||
| } | ||
| return 0; | ||
| } |
Comment on lines
+311
to
+314
| TEST_F(GDialPlatAppTest, RegisterApplications_ValidPtr_ReturnsNone) { | ||
| int dummy = 42; | ||
| EXPECT_EQ(gdial_plat_application_register_applications(&dummy), GDIAL_APP_ERROR_NONE); | ||
| } |
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.