RDKC-16494: Migrate crashupload to native C binary for RDKC camera platform#66
RDKC-16494: Migrate crashupload to native C binary for RDKC camera platform#66rdkcteam wants to merge 11 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Note
Copilot was unable to run its full agentic suite in this review.
Adds support for the new DEVICE_TYPE_XHC1 (RDKC camera) across crashupload config detection, archiving, RFC access, and upload flows (including a non‑mTLS path intended to match uploadDumps.sh behavior).
Changes:
- Introduces
DEVICE_TYPE_XHC1and detects it from device properties; adjusts default paths/box type for RDKC camera. - Adds XHC1-specific archive composition and upload flow (plain HTTPS POST/PUT without mTLS).
- Adapts RFC interface for RDKC’s RFC API variant and adds partner ID retrieval from a file.
Reviewed changes
Copilot reviewed 7 out of 8 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| c_sourcecode/src/utils/prerequisites.c | Extends upload deferral logic to include DEVICE_TYPE_XHC1. |
| c_sourcecode/src/upload/upload.c | Adds XHC1 non‑mTLS upload path, RDKC partner ID file read, and XHC1 portal/S3 URL behavior. |
| c_sourcecode/src/rfcInterface/rfcinterface.h | Adds RDKC-specific RFC function prototypes with const char *type. |
| c_sourcecode/src/rfcInterface/rfcinterface.c | Implements RDKC RFC read/write behavior using getRFCParameter. |
| c_sourcecode/src/config/config_manager.c | Detects XHC1 and overrides core paths and box type defaults for RDKC camera. |
| c_sourcecode/src/archive/archive.c | Adds XHC1-specific tarball file list for minidump archives. |
| c_sourcecode/common/types.h | Adds DEVICE_TYPE_XHC1 to device_type_t. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| data_len = strlen(param.value); | ||
| if (data_len >= 2 && (param.value[0] == '"') && (param.value[data_len - 1] == '"')) | ||
| { | ||
| // remove quotes around data | ||
| snprintf(out_value, datasize, "%s", ¶m.value[1]); | ||
| *(out_value + data_len - 2) = 0; | ||
| } |
There was a problem hiding this comment.
The terminator index is now clamped: min(data_len - 2, datasize - 1) — so if snprintf truncated the output, the null-terminator write stays within the out_value buffer bounds.
| int read_RFCProperty(const char *type, const char *key, char *out_value, size_t datasize) | ||
| { | ||
| RFC_ParamData_t param; | ||
| int data_len; |
There was a problem hiding this comment.
Added (void)type; in read_RFCProperty (only unused param there) and (void)type; (void)key; (void)value; (void)datatype; in write_RFCProperty (all params unused).
| file_upload.url = (char *)url; | ||
| file_upload.pathname = s3_url_file; | ||
| file_upload.pPostFields = post_filed; | ||
| file_upload.sslverify = 0; |
There was a problem hiding this comment.
OCSP stapling is disabled because the S3 signing endpoint does not provide stapled OCSP responses for RDKC platform
| { | ||
| FileUpload_t file_upload; | ||
| memset(&file_upload, 0, sizeof(FileUpload_t)); | ||
| file_upload.url = (char *)url; |
There was a problem hiding this comment.
url is read-only downstream (passed to curl_easy_setopt).FileUpload_t.url is char* for API compatibility; not mutated.
| ret = performS3PutUpload(out_url, filepath, | ||
| (device_type == DEVICE_TYPE_XHC1) ? NULL : &sec_out); |
There was a problem hiding this comment.
performS3PutUpload explicitly guards the auth parameter with if (auth) before calling setMtlsHeaders. When NULL is passed, mTLS setup is simply skipped. No crash risk.
| char receiver_log[64] = {0}; | ||
| char thread_log[64] = {0}; | ||
| snprintf(receiver_log, sizeof(receiver_log), "%s/receiver.log", config->log_path); | ||
| snprintf(thread_log, sizeof(thread_log), "%s/thread.log", config->log_path); |
| DEVICE_TYPE_VIDEO, | ||
| DEVICE_TYPE_EXTENDER, | ||
| DEVICE_TYPE_MEDIACLIENT, | ||
| DEVICE_TYPE_XHC1, |
There was a problem hiding this comment.
Change as RDKC or CAMERA and avoid using modelname
| if (strcmp(config->box_type, "UNKNOWN") == 0 && | ||
| config->device_type == DEVICE_TYPE_XHC1) | ||
| { | ||
| strcpy(config->box_type, "xcam"); |
|
All contributors have signed the CLA ✍️ ✅ |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 8 changed files in this pull request and generated 3 comments.
Comments suppressed due to low confidence (1)
c_sourcecode/src/upload/upload.c:404
- On RDKC the PartnerId comes from a file, but the log messages still say "GetPartnerId ..." (including the failure path). This is misleading for debugging and telemetry because GetPartnerId() isn't called under
#ifdef RDKC.
if (ret == 0)
{
strcpy(pPartnerId, "comcast");
CRASHUPLOAD_ERROR("GetPartnerId is failed. Assign default:%s\n", pPartnerId);
}
else
{
CRASHUPLOAD_INFO("GetPartnerId is Success:%s\n", pPartnerId);
}
| @@ -147,6 +154,20 @@ int config_init_load(config_t *config, int argc, char *argv[]) | |||
| strcpy(config->core_path, "/var/lib/systemd/coredump"); | |||
| strcpy(config->minidump_path, "/opt/minidumps"); | |||
| } | |||
| /* Override paths for non-systemd RDKC camera */ | |||
| if (config->upload_mode == UPLOAD_MODE_NORMAL && | |||
| config->device_type == DEVICE_TYPE_RDKC) | |||
| { | |||
| strcpy(config->core_path, "/opt/corefiles"); | |||
| /* minidump_path remains /opt/minidumps - already correct for RDKC */ | |||
| } | |||
| /* BOX_TYPE fallback for RDKC camera (no BOX_TYPE in device.properties) */ | |||
| if (strcmp(config->box_type, "UNKNOWN") == 0 && | |||
| config->device_type == DEVICE_TYPE_RDKC) | |||
| { | |||
| strcpy(config->box_type, "xcam"); | |||
| CRASHUPLOAD_INFO("BOX_TYPE defaulted to xcam for RDKC\n"); | |||
| } | |||
| else if (config->device_type == DEVICE_TYPE_RDKC) | ||
| { | ||
| /* RDKC Camera minidump archive: | ||
| * Matches script uploadDumps.sh line 917 for DEVICE_TYPE=RDKC | ||
| * Archive: dumpName + version.txt + core_log.txt | ||
| * Non-prod: also receiver.log and thread.log if present | ||
| */ | ||
| snprintf(arch_files_list[0], 256, "%s", new_dump_name); | ||
| snprintf(arch_files_list[1], 256, "%s", "/version.txt"); | ||
| snprintf(arch_files_list[2], 256, "%s", config->core_log_file); | ||
|
|
||
| if (config->build_type != BUILD_TYPE_PROD) | ||
| { | ||
| char receiver_log[PATH_MAX] = {0}; | ||
| char thread_log[PATH_MAX] = {0}; | ||
| snprintf(receiver_log, sizeof(receiver_log), "%s/receiver.log", config->log_path); | ||
| snprintf(thread_log, sizeof(thread_log), "%s/thread.log", config->log_path); | ||
| if (0 == (filePresentCheck(receiver_log))) | ||
| { | ||
| snprintf(arch_files_list[no_of_files], 256, "%s", receiver_log); | ||
| CRASHUPLOAD_INFO("RDKC adding receiver.log=%s\n", arch_files_list[no_of_files]); | ||
| no_of_files++; | ||
| } | ||
| if (0 == (filePresentCheck(thread_log))) | ||
| { | ||
| snprintf(arch_files_list[no_of_files], 256, "%s", thread_log); | ||
| CRASHUPLOAD_INFO("RDKC adding thread.log=%s\n", arch_files_list[no_of_files]); | ||
| no_of_files++; | ||
| } | ||
| } | ||
| CRASHUPLOAD_INFO("RDKC tar file:%s files=%d\n", target_file_name, no_of_files); | ||
| tar_status = create_tarball(true, target_file_name, arch_files_list, no_of_files); | ||
| } |
| @@ -422,6 +444,25 @@ int upload_process(archive_info_t *archive, const config_t *config, const platfo | |||
| CRASHUPLOAD_INFO("Read rfc Success crashportalEndpointUrl:\n Overriding the S3 Amazon Signing URL:%s\n", crashportalEndpointUrl); | |||
| } | |||
| } | |||
| else if (config->device_type == DEVICE_TYPE_RDKC) | |||
| { | |||
| /* RDKC Camera upload flow: | |||
| * Matches script uploadDumps.sh - encryptionEnable=false (no /etc/os-release) | |||
| * S3 URL from device.properties S3_AMAZON_SIGNING_URL | |||
| * Portal URL: crashportal.stb.r53.xcal.tv | |||
| */ | |||
| strcpy(encryptionEnable, "false"); | |||
| strcpy(portal_url, "crashportal.stb.r53.xcal.tv"); | |||
| request_type = 17; | |||
| CRASHUPLOAD_INFO("RDKC: request_type=%d\n", request_type); | |||
| ret = get_crashupload_s3signed_url(crashportalEndpointUrl, sizeof(crashportalEndpointUrl)); | |||
| if (ret < 0) | |||
| { | |||
| CRASHUPLOAD_ERROR("RDKC: Unable to get the S3 server url. So exit\n"); | |||
| return ret; | |||
| } | |||
| CRASHUPLOAD_INFO("RDKC: S3 signing URL=%s\n", crashportalEndpointUrl); | |||
| } | |||
satya200
left a comment
There was a problem hiding this comment.
Please schedule a call for more discussion
| #if defined(RDKC) | ||
| /* RDKC uses 2-param getRFCParameter that reads from ini files. | ||
| * Returns int: SUCCESS(0) or FAILURE(1). No setRFCParameter available. */ | ||
| int read_RFCProperty(const char *type, const char *key, char *out_value, size_t datasize) |
There was a problem hiding this comment.
Why you need same duplicate function?
| #define RFC_CRASH_PORTAL_ENDPOINT_URL "Device.DeviceInfo.X_RDKCENTRAL-COM_RFC.Feature.CrashportalEndpoint.URL" | ||
|
|
||
| #if defined(RFC_API_ENABLED) | ||
| #if defined(RDKC) |
There was a problem hiding this comment.
Why you need. Already these functions are declared and defined
There was a problem hiding this comment.
the RDKC rfcapi.h would need 2-param getRFCParameter signature as not like the STB version 3-param — that's a difference there in the rfc library itself which forces us to make to do change in crashupload.
| #include "mtls_upload.h" | ||
| #include "uploadUtil.h" | ||
| #include "downloadUtil.h" | ||
| #include "upload_status.h" | ||
| #include "ratelimit.h" |
| - S3 signed URL from RFC (`CrashUpload.S3SigningUrl`) or device.properties (`S3_AMAZON_SIGNING_URL`) | ||
| - Request type: 17 | ||
| - Portal: `crashportal.stb.r53.xcal.tv` | ||
| - Encryption: disabled (`encryptionEnable=false`) | ||
| - Authentication: mTLS via `rdkcertselector` (cert rotation supported) | ||
| - **RFC Interface**: 2-parameter `getRFCParameter` variant reading from INI files; `setRFCParameter` not supported |
|
I have read the CLA Document and I hereby sign the CLA |
Replaces shell-based uploadDumps.sh with a native C binary on RDKC devices. All devices use mTLS cert rotation via rdkcertselector . Partner ID is read from /opt/usr_config/partnerid.txt under compile-time RDKC flag. RFC parameters sourced via 2-param ini-file API. Archive creation uses libarchive for tar.gz with nice -19. No regression on non-RDKC platforms — preserved behind runtime device_type checks and compile-time #ifdef RDKC guards.