diff --git a/CHANGELOG.md b/CHANGELOG.md index 8fd15ba9..f3d94520 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,9 +4,22 @@ All notable changes to this project will be documented in this file. Dates are d Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog). +#### [1.6.2](https://github.com/rdkcentral/rdkfwupdater/compare/1.6.1...1.6.2) + +- RDKEMW-10029 : Syncing of Gerrit commits that are required for security components [`#209`](https://github.com/rdkcentral/rdkfwupdater/pull/209) +- RDKEMW-15146 : Introduce mechanism to unlock debug service - read /opt/swupdate.conf [`#205`](https://github.com/rdkcentral/rdkfwupdater/pull/205) +- RDKTV-39830:Xumo TV devices got stuck with old firmware with http eror code 405 [`#213`](https://github.com/rdkcentral/rdkfwupdater/pull/213) +- Enable L2 For PKCS11 [`#192`](https://github.com/rdkcentral/rdkfwupdater/pull/192) +- RDKTV-39830:Xumo TV devices got stuck with old firmware with http error code 405- adding L1 tests [`3b9880b`](https://github.com/rdkcentral/rdkfwupdater/commit/3b9880b43a75053f159a8dfc20c874277eb83f63) +- RDKTV-39830:Xumo TV devices got stuck with old firmware with http error code 405 [`3beb5c8`](https://github.com/rdkcentral/rdkfwupdater/commit/3beb5c8638ed87bb500985d651a9b24b34aea0e1) +- RDKTV-39830:Xumo TV devices got stuck with old firmware with http error code 405 [`e261e90`](https://github.com/rdkcentral/rdkfwupdater/commit/e261e90b596f12c28be561e0d132059bc95cc2e7) + #### [1.6.1](https://github.com/rdkcentral/rdkfwupdater/compare/1.6.0...1.6.1) +> 11 March 2026 + - RDK-60527:Implement registerProcess and unregisterProcess APIs [`#194`](https://github.com/rdkcentral/rdkfwupdater/pull/194) +- 1.6.1 release changelog updates [`c1d0d29`](https://github.com/rdkcentral/rdkfwupdater/commit/c1d0d2983a68708984637eddce9d8ae4b9c436c2) - Merge tag '1.6.0' into develop [`cca2cb9`](https://github.com/rdkcentral/rdkfwupdater/commit/cca2cb9c2faaa01e3f9f39ddcdf71a77b1e1c717) #### [1.6.0](https://github.com/rdkcentral/rdkfwupdater/compare/1.5.2...1.6.0) diff --git a/Makefile.am b/Makefile.am index 58363925..02772f88 100644 --- a/Makefile.am +++ b/Makefile.am @@ -181,7 +181,6 @@ include_HEADERS = \ librdkFwupdateMgr_la_SOURCES = \ ${top_srcdir}/librdkFwupdateMgr/src/rdkFwupdateMgr_process.c \ - ${top_srcdir}/librdkFwupdateMgr/src/rdkFwupdateMgr_log.c \ ${top_srcdir}/librdkFwupdateMgr/src/rdkFwupdateMgr_async.c \ ${top_srcdir}/librdkFwupdateMgr/src/rdkFwupdateMgr_api.c @@ -258,13 +257,17 @@ example_plugin_SOURCES = \ example_plugin_CFLAGS = \ -I${top_srcdir}/librdkFwupdateMgr/include \ + -I${top_srcdir}/librdkFwupdateMgr/src \ + -I${top_srcdir}/common_utilities/utils \ $(AM_CFLAGS) \ $(GLIB_CFLAGS) example_plugin_LDADD = \ librdkFwupdateMgr.la \ $(GLIB_LIBS) \ - -lpthread + -lpthread \ + -lfwutils \ + -lrdkloggers example_plugin_LDFLAGS = \ -L$(PKG_CONFIG_SYSROOT_DIR)/$(libdir) diff --git a/librdkFwupdateMgr/docs/DESIGN_DOCUMENT.md b/librdkFwupdateMgr/docs/DESIGN_DOCUMENT.md new file mode 100755 index 00000000..3b4a8c0a --- /dev/null +++ b/librdkFwupdateMgr/docs/DESIGN_DOCUMENT.md @@ -0,0 +1,1385 @@ +# librdkFwupdateMgr — Engineering Design Document + +> **Document Version**: 2.0 +> **Date**: May 02, 2026 +> **Component**: `librdkFwupdateMgr` (shared library) + +--- + +## Table of Contents + +1. [Executive Summary](#1-executive-summary) +2. [Why This Library Exists](#2-why-this-library-exists) +3. [Why Clients Must Not Directly Talk to the Daemon](#3-why-clients-must-not-directly-talk-to-the-daemon) +4. [Shared Library Responsibilities](#4-shared-library-responsibilities) +5. [Daemon Responsibilities](#5-daemon-responsibilities) +6. [High-Level Request Lifecycle](#6-high-level-request-lifecycle) +7. [Detailed Public API Reference](#7-detailed-public-api-reference) + - 7.1 [registerProcess()](#71-registerprocess) + - 7.2 [checkForUpdate()](#72-checkforupdate) + - 7.3 [downloadFirmware()](#73-downloadfirmware) + - 7.4 [updateFirmware()](#74-updatefirmware) + - 7.5 [unregisterProcess()](#75-unregisterprocess) +8. [Internal Helper Modules](#8-internal-helper-modules) +9. [IPC Communication Model](#9-ipc-communication-model) +10. [Retry and Timeout Strategy](#10-retry-and-timeout-strategy) +11. [Failure Scenarios and Recovery](#11-failure-scenarios-and-recovery) +12. [Logging Architecture](#12-logging-architecture) +13. [Security Considerations](#13-security-considerations) +14. [Performance Considerations](#14-performance-considerations) +15. [Scalability for Multiple Clients](#15-scalability-for-multiple-clients) +16. [Future Extensibility](#16-future-extensibility) + +--- + +## 1. Executive Summary + +`librdkFwupdateMgr` is a C shared library (`librdkFwupdateMgr.so`) that provides client applications on RDK-based embedded devices a clean, stable API for firmware lifecycle management. The library acts as a thin, intelligent client-side proxy that communicates with the `rdkFwupdateMgr` daemon over D-Bus (system bus IPC). + +### What Problem Does It Solve? + +Firmware updates on embedded devices require coordination between: +- Cloud infrastructure (XConf configuration server) +- Local device services (download managers, flash subsystems, reboot coordinators) +- Multiple client applications that may each need to initiate or observe firmware updates + +Without this library, every client application would need to: +- Know D-Bus method names, object paths, interface names, and GVariant type signatures +- Manage D-Bus connections, proxies, and signal subscriptions +- Handle threading for asynchronous signal reception +- Implement timeout and retry logic +- Parse raw D-Bus data into usable structures + +This library eliminates all of that complexity. A client app includes one header, links one library, calls five functions, and implements three callback signatures. + +### Key Design Characteristics + +| Characteristic | Decision | Rationale | +|---------------|----------|-----------| +| IPC mechanism | D-Bus (system bus) | Standard Linux IPC, D-Bus policy enforcement, well-understood | +| Threading model | Single background thread + per-call ephemeral connections | Simple, minimal resource usage | +| API style | Async fire-and-forget with callbacks | Non-blocking; suitable for event-driven and threaded apps | +| Memory model | Library owns handle; client owns callback data copies | Clear ownership boundaries | +| Connection model | Stateless (new D-Bus connection per API call) | No connection lifecycle management needed | +| Callback delivery | Background thread invocation | Deterministic delivery; client uses condvar to synchronize | + +--- + +## 2. Why This Library Exists + +### 2.1 The Fundamental Problem + +The `rdkFwupdateMgr` daemon manages all firmware operations on the device: checking for updates, downloading firmware images, flashing them to storage, and coordinating reboots. Multiple client applications need to interact with this daemon: + +- **example_plugin** — A reference one-shot firmware updater +- **TR-069/TR-181 agents** — Remote management protocols that trigger updates +- **WebUI services** — User-facing interfaces showing update status +- **Monitoring daemons** — Health-check services that poll firmware state + +Each of these would need to implement identical D-Bus client logic if they talked to the daemon directly. This is the classic "N-clients × M-operations" maintenance problem. + +### 2.2 What the Library Provides + +``` +WITHOUT library: WITH library: +───────────────── ───────────────── +Client A: 200 lines D-Bus code Client A: 30 lines using 5 API calls +Client B: 200 lines D-Bus code Client B: 30 lines using 5 API calls +Client C: 200 lines D-Bus code Client C: 30 lines using 5 API calls + Library: 1500 lines (maintained once) +``` + +### 2.3 Design Goals + +1. **Simplicity** — Five public functions. Three callback types. One header to include. +2. **Correctness** — Thread-safe, leak-free, handles all error paths. +3. **Stability** — Public API (header) changes require version bumps. Internal implementation can change freely. +4. **Observability** — Structured logging with module separation (`[FWUPMGR]`). +5. **Portability** — Works with or without RDK_LOGGER. Falls back to `fprintf` for unit testing. + +--- + +## 3. Why Clients Must Not Directly Talk to the Daemon + +This section documents the engineering rationale for mandating library usage rather than allowing direct D-Bus calls. + +### 3.1 Protocol Encapsulation + +The D-Bus interface between client and daemon is an **internal protocol**, not a public contract: + +| Aspect | Risk of Direct D-Bus Access | +|--------|---------------------------| +| Method signatures | `(ss) → (t)`, `(ssss)`, `(sssss)` — one typo = crash or silent failure | +| Signal signatures | `(tiissss)`, `(tsuss)`, `(tsiis)` — must parse correctly or lose data | +| Signal subscription setup | Must happen BEFORE method call, or response signal is lost | +| Update details format | Pipe-separated `Key:Value` string — undocumented, may change | +| Handler ID encoding | `uint64` on wire but `char*` in API — format details shouldn't leak | + +If the daemon team changes a signal signature (e.g., adds a field), only the library needs updating — not every client. + +### 3.2 Threading Complexity + +Receiving D-Bus signals requires: +- A dedicated GLib event loop running in a thread +- Signal subscription with correct object path and interface filters +- Proper GMainContext isolation (so the app's own GLib loop isn't disrupted) +- Mutex-protected callback dispatch with deadlock prevention + +No client developer should reimplement this. It's error-prone and already solved in the library. + +### 3.3 Connection Lifecycle + +The daemon identifies clients by `handler_id`, not by D-Bus sender address. This is because our library's stateless model creates a **new D-Bus connection per API call**, meaning each call gets a different sender ID (`:1.140`, `:1.141`, `:1.145`, etc.). + +A naive client attempting direct D-Bus calls would likely assume sender-ID stability — leading to authorization failures at the daemon's process tracking layer. + +### 3.4 Forward Compatibility + +The library provides a stable ABI boundary: +- Daemon protocol changes → library absorbs them internally +- New capabilities (e.g., cancel, pause) → added as new library functions +- Client code recompiles against same header, links same `.so` name + +--- + +## 4. Shared Library Responsibilities + +The library (`librdkFwupdateMgr.so`) is responsible for the following and **only** the following: + +| # | Responsibility | Implementation | +|---|---------------|----------------| +| 1 | **Input validation** | NULL checks, empty string checks, length limits on all public API parameters | +| 2 | **D-Bus transport** | Create connection, build GVariant payloads, send method calls, handle D-Bus errors | +| 3 | **Async signal reception** | Background thread with GLib event loop subscribed to daemon signals | +| 4 | **Callback management** | Three registries (check, download, update) with mutex protection | +| 5 | **Data transformation** | Convert daemon's wire format (GVariant) into typed C structs (`FwInfoData`, progress values) | +| 6 | **Handle lifecycle** | Allocate on register, validate on use, free on unregister | +| 7 | **Structured logging** | All operations logged under `LOG.RDK.FWUPMGR` module | +| 8 | **Resource cleanup** | Thread join, mutex destroy, memory free on deinit — no leaks | + +### What the Library Does NOT Do + +- Does NOT perform actual firmware downloads (daemon does this) +- Does NOT interact with XConf servers (daemon does this) +- Does NOT flash firmware (daemon delegates to HAL) +- Does NOT manage reboots (daemon handles this) +- Does NOT persist state across process restarts +- Does NOT retry failed operations (client's responsibility) +- Does NOT own the logging lifecycle (`log_init`/`log_exit` are caller's job) + +--- + +## 5. Daemon Responsibilities + +For context, here is what the `rdkFwupdateMgr` daemon does (the other side of the D-Bus): + +| # | Responsibility | Details | +|---|---------------|---------| +| 1 | **Process registration** | Assign unique handler_id, track client metadata, enforce one-registration-per-process-name | +| 2 | **XConf query** | HTTP request to XConf server with device model, current firmware version, MAC address | +| 3 | **Firmware download** | HTTP/HTTPS download from CDN, chunked transfer, integrity verification | +| 4 | **Flash coordination** | Write firmware to appropriate storage partition via device HAL | +| 5 | **Progress reporting** | Emit D-Bus signals with percentage and status as operations progress | +| 6 | **Multi-client orchestration** | Coalesce duplicate download requests (piggybacking), serialize flash operations | +| 7 | **Reboot management** | Coordinate post-flash reboot timing based on client's `rebootImmediately` flag | +| 8 | **Cleanup on client disconnect** | Handle ungraceful client exits (D-Bus name owner watching) | + +--- + +## 6. High-Level Request Lifecycle + +Every firmware update workflow follows this lifecycle: + +``` +┌─────────────────────────────────────────────────────────────────────┐ +│ CLIENT APPLICATION │ +│ │ +│ ┌──────────┐ ┌──────────────┐ ┌────────────┐ ┌──────┐│ +│ │ REGISTER │────►│ CHECK UPDATE │────►│ DOWNLOAD │────►│FLASH ││ +│ └──────────┘ └──────────────┘ └────────────┘ └──────┘│ +│ │ │ │ │ │ +│ │ callback fires callback fires callback │ +│ │ (once, with (many times, fires │ +│ │ firmware info) with progress) (many) │ +│ │ │ +│ ┌────────────┐ │ +│ │ UNREGISTER │◄───────────────────── (always, even on error) ────┘│ +│ └────────────┘ │ +└─────────────────────────────────────────────────────────────────────┘ +``` + +### Lifecycle States + +``` + ┌────────────┐ + │ UNLINKED │ Library loaded but not registered + └─────┬──────┘ + │ registerProcess() succeeds + ▼ + ┌────────────┐ + │ REGISTERED │ Handle valid, background thread running, ready for API calls + └─────┬──────┘ + │ checkForUpdate() / downloadFirmware() / updateFirmware() + ▼ + ┌────────────┐ + │ ACTIVE │ One or more async operations pending + └─────┬──────┘ + │ All callbacks have fired (completed/errored) + ▼ + ┌────────────┐ + │ REGISTERED │ Back to idle, can call APIs again + └─────┬──────┘ + │ unregisterProcess() + ▼ + ┌────────────┐ + │ UNLINKED │ Handle freed, thread stopped, library dormant + └────────────┘ +``` + +### Complete Sequence (Normal Path) + +``` +Time Client App librdkFwupdateMgr Daemon +───── ────────── ───────────────── ────── +T+0s registerProcess() + ├─────────────────── D-Bus: RegisterProcess ──────────► assigns handler_id + │◄─────────────────── returns handler_id ──────────────┘ + ├─ internal_system_init() + │ └─ spawns background thread + │ ├─ subscribes to 3 signals + │ └─ enters g_main_loop_run() + └─ returns handle "12345" + +T+1s checkForUpdate(handle, cb) + ├─ validates inputs + ├─ registers cb in registry + ├────────────── D-Bus: CheckForUpdate ──────────────────► queries XConf + └─ returns SUCCESS immediately + +T+15s XConf response arrives + ◄── CheckForUpdateComplete ────┤ + dispatches cb(&fwinfo) + └─► cb runs in BG thread + +T+16s downloadFirmware(handle, req, dl_cb) + ├─ validates inputs + ├─ registers dl_cb in dwnl registry + ├────────── D-Bus: DownloadFirmware ────────────────────► starts download + └─ returns SUCCESS + +T+20s download 25% + ◄── DownloadProgress(25%) ─────┤ + dispatches dl_cb(25, IN_PROGRESS) +T+40s download 100% + ◄── DownloadProgress(100%) ────┤ + dispatches dl_cb(100, COMPLETED) + resets slot to IDLE + +T+41s updateFirmware(handle, req, upd_cb) + ├─ validates inputs + ├─ registers upd_cb in update registry + ├─────────── D-Bus: UpdateFirmware ─────────────────────► starts flash + └─ returns SUCCESS + +T+90s flash 100% + ◄── UpdateProgress(100%) ──────┤ + dispatches upd_cb(100, COMPLETED) + +T+91s unregisterProcess(handle) + ├─ internal_system_deinit() + │ ├─ g_main_loop_quit() + │ ├─ pthread_join() + │ └─ free registries + mutexes + ├────────── D-Bus: UnregisterProcess ───────────────────► removes ProcessInfo + └─ free(handle) +``` + +--- + +## 7. Detailed Public API Reference + +### 7.1 `registerProcess()` + +```c +FirmwareInterfaceHandle registerProcess(const char *processName, const char *libVersion); +``` + +#### Purpose + +Establishes a session with the firmware daemon. This is the mandatory first call before any other API can be used. It tells the daemon "I exist, here's my name, give me a session token." + +#### Inputs + +| Parameter | Type | Constraints | Example | +|-----------|------|-------------|---------| +| `processName` | `const char*` | Non-NULL, non-empty, ≤256 chars | `"VideoPlayer"` | +| `libVersion` | `const char*` | Non-NULL, ≤64 chars (empty OK) | `"1.0.0"` | + +#### Output + +| Return | Meaning | +|--------|---------| +| Non-NULL string (e.g., `"12345"`) | Success — this is your session handle | +| `NULL` | Failure — daemon not running, D-Bus error, or validation failure | + +#### Return Code Semantics + +This function doesn't use an enum return code — it returns the handle directly or `NULL`. + +#### Internal Steps (What Happens Inside) + +``` +Step 1: Validate processName + ├─ NULL check → FWUPMGR_ERROR, return NULL + ├─ Empty check → FWUPMGR_ERROR, return NULL + └─ Length check (>256) → FWUPMGR_ERROR, return NULL + +Step 2: Validate libVersion + ├─ NULL check → FWUPMGR_ERROR, return NULL + └─ Length check (>64) → FWUPMGR_ERROR, return NULL + +Step 3: Create D-Bus proxy + ├─ g_bus_get_sync(G_BUS_TYPE_SYSTEM) → GDBusConnection + │ └─ Failure: log error, return NULL + ├─ g_dbus_proxy_new_sync() → GDBusProxy + │ └─ Failure: log error, unref connection, return NULL + └─ Unref connection (proxy holds its own reference) + +Step 4: Call RegisterProcess D-Bus method + ├─ g_dbus_proxy_call_sync("RegisterProcess", (ss), timeout=5000ms) + │ └─ Failure: log D-Bus error message, unref proxy, return NULL + ├─ Extract handler_id (uint64) from reply GVariant (t) + └─ Unref result + proxy + +Step 5: Allocate handle string + ├─ malloc(32) → buffer for decimal string + │ └─ Failure: CRITICAL — registration succeeded but can't return handle + │ ├─ Create cleanup proxy + │ ├─ Call UnregisterProcess(handler_id) to undo daemon-side registration + │ └─ Return NULL + └─ snprintf(buffer, 32, "%" PRIu64, handler_id) + +Step 6: Start async engine + ├─ internal_system_init() + │ ├─ Initialize CallbackRegistry (mutex + zero array) + │ ├─ Initialize DwnlCallbackRegistry (mutex + zero array) + │ ├─ Initialize UpdateCbRegistry (mutex + zero array) + │ ├─ Create GMainContext (isolated from app's GLib) + │ ├─ Create GMainLoop + │ ├─ pthread_create(background_thread_func) + │ └─ Spin-wait (max 5s) until bg thread sets running=true + └─ Return handle string +``` + +#### Error Handling + +| Error Condition | Action | User-Visible Effect | +|----------------|--------|---------------------| +| NULL/empty processName | Log error, return NULL immediately | No D-Bus call made | +| D-Bus system bus unavailable | Log connection error, return NULL | Daemon may not be installed | +| Daemon not responding (timeout) | Log timeout, return NULL | Daemon may be crashed/overloaded | +| Daemon rejects registration | Log D-Bus error message, return NULL | Process name conflict or internal error | +| malloc failure after success | Best-effort UnregisterProcess, return NULL | Extremely rare (OOM condition) | +| Background thread fails to start | Log error, return NULL | System resource exhaustion | + +#### Thread Safety + +- Fully thread-safe for concurrent calls (different process names) +- GDBus synchronous calls are internally thread-safe +- No shared state until `internal_system_init()` creates registries (which are mutex-protected) + +#### Memory Ownership + +| Who | Owns What | +|-----|-----------| +| Library | The returned handle string (malloc'd) | +| Caller | NOTHING — do not free the handle; call `unregisterProcess()` instead | + +#### Typical Caller Usage + +```c +#include "rdkFwupdateMgr_client.h" + +int main(void) { + FirmwareInterfaceHandle handle = registerProcess("MyPlugin", "2.1.0"); + if (handle == NULL) { + fprintf(stderr, "Failed to register with daemon. Is it running?\n"); + return EXIT_FAILURE; + } + + printf("Registered! Handle: %s\n", handle); + + // ... use other APIs with this handle ... + + unregisterProcess(handle); // MUST call this before exit + return EXIT_SUCCESS; +} +``` + +--- + +### 7.2 `checkForUpdate()` + +```c +CheckForUpdateResult checkForUpdate(FirmwareInterfaceHandle handle, UpdateEventCallback callback); +``` + +#### Purpose + +Initiates a non-blocking firmware availability check. The daemon queries the XConf cloud server to determine if a newer firmware version exists for this device. The result is delivered asynchronously via your callback — this function returns immediately. + +#### Inputs + +| Parameter | Type | Constraints | Example | +|-----------|------|-------------|---------| +| `handle` | `FirmwareInterfaceHandle` | Non-NULL, non-empty, from `registerProcess()` | `"12345"` | +| `callback` | `UpdateEventCallback` | Non-NULL function pointer | `my_check_callback` | + +#### Callback Signature + +```c +typedef void (*UpdateEventCallback)(const FwInfoData *fwinfodata); +``` + +The callback receives: +```c +typedef struct { + char CurrFWVersion[64]; // Current firmware version on device + UpdateDetails *UpdateDetails; // Non-NULL only when status == FIRMWARE_AVAILABLE + CheckForUpdateStatus status; // Result enum +} FwInfoData; + +typedef struct { + char FwFileName[128]; // e.g., "firmware_v2.0.bin" + char FwUrl[512]; // Download URL + char FwVersion[64]; // Available firmware version + char RebootImmediately[12]; // "true" or "false" + char DelayDownload[8]; // "true" or "false" + char PDRIVersion[64]; // PDRI image version (may be empty) + char PeripheralFirmwares[256]; // Peripheral versions (may be empty) +} UpdateDetails; +``` + +#### Output + +| Return Value | Meaning | +|-------------|---------| +| `CHECK_FOR_UPDATE_SUCCESS` (0) | Request sent to daemon. Callback will fire later. | +| `CHECK_FOR_UPDATE_FAIL` (1) | Request could not be sent. No callback will fire. | + +**Critical**: `SUCCESS` does NOT mean firmware is available. It means the request was accepted. Actual firmware availability comes through the callback. + +#### Internal Steps + +``` +Step 1: Validate handle (not NULL, not empty) → FAIL on error +Step 2: Validate callback (not NULL) → FAIL on error +Step 3: Connect to D-Bus system bus + └─ Failure: return FAIL (no stale registry entry created) +Step 4: Register callback in CallbackRegistry (slot: IDLE → PENDING) + └─ Failure (registry full, 30 slots): unref connection, return FAIL +Step 5: Fire-and-forget g_dbus_connection_call("CheckForUpdate", (s)handle) + └─ No reply expected — returns immediately +Step 6: Unref D-Bus connection +Step 7: Return CHECK_FOR_UPDATE_SUCCESS +``` + +#### Error Handling + +| Error | Action | Callback Fires? | +|-------|--------|----------------| +| Invalid handle | Return FAIL immediately | No | +| NULL callback | Return FAIL immediately | No | +| D-Bus connection failure | Return FAIL, no registry entry | No | +| Registry full (30 slots) | Return FAIL, cleanup connection | No | +| Daemon crashes after call sent | Callback never fires (client should timeout) | No — client uses condvar timeout | + +#### Thread Safety + +- Safe to call from multiple threads concurrently (registry mutex protects slot allocation) +- Same handle can have only one pending check at a time (existing slot is overwritten) +- Callback fires in the library's background thread, NOT the caller's thread + +#### Memory Ownership + +| Data | Lifetime | Owner | +|------|----------|-------| +| `FwInfoData*` passed to callback | Valid ONLY during callback execution | Library (stack-allocated in dispatch function) | +| `UpdateDetails*` inside FwInfoData | Valid ONLY during callback execution | Library (stack-allocated) | +| All strings in FwInfoData | Valid ONLY during callback execution | Library | + +**Rule**: If you need data after the callback returns, `strncpy()` it to your own buffers before the callback returns. + +#### Typical Caller Usage + +```c +static pthread_mutex_t check_mutex = PTHREAD_MUTEX_INITIALIZER; +static pthread_cond_t check_cond = PTHREAD_COND_INITIALIZER; +static int check_done = 0; +static CheckForUpdateStatus check_result; + +void my_check_callback(const FwInfoData *fwinfo) { + // This runs in BACKGROUND THREAD — copy what you need, signal main thread + pthread_mutex_lock(&check_mutex); + check_result = fwinfo->status; + check_done = 1; + pthread_cond_signal(&check_cond); + pthread_mutex_unlock(&check_mutex); +} + +// In main thread: +CheckForUpdateResult rc = checkForUpdate(handle, my_check_callback); +if (rc != CHECK_FOR_UPDATE_SUCCESS) { + // Handle error — callback will NOT fire + return; +} + +// Wait for callback with 2-minute timeout +struct timespec timeout; +clock_gettime(CLOCK_REALTIME, &timeout); +timeout.tv_sec += 120; + +pthread_mutex_lock(&check_mutex); +while (!check_done) { + if (pthread_cond_timedwait(&check_cond, &check_mutex, &timeout) != 0) { + // Timeout — XConf query took too long + break; + } +} +pthread_mutex_unlock(&check_mutex); +``` + +--- + +### 7.3 `downloadFirmware()` + +```c +DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, + const FwDwnlReq *fwdwnlreq, + DownloadCallback callback); +``` + +#### Purpose + +Initiates a non-blocking firmware image download. The daemon downloads the specified firmware file from the CDN and reports progress through repeated callback invocations. Returns immediately. + +#### Inputs + +| Parameter | Type | Constraints | +|-----------|------|-------------| +| `handle` | `FirmwareInterfaceHandle` | Non-NULL, non-empty | +| `fwdwnlreq` | `const FwDwnlReq*` | Non-NULL; `firmwareName` must be non-NULL and non-empty | +| `callback` | `DownloadCallback` | Non-NULL | + +```c +typedef struct { + const char *firmwareName; // REQUIRED: "firmware_v2.bin" + const char *downloadUrl; // OPTIONAL: NULL or "" → daemon uses XConf URL + const char *TypeOfFirmware; // OPTIONAL: "PCI", "PDRI", or "PERIPHERAL" +} FwDwnlReq; +``` + +#### Callback Signature + +```c +typedef void (*DownloadCallback)(int download_progress, DownloadStatus fwdwnlstatus); +``` + +Called **multiple times**: +- `(10, DWNL_IN_PROGRESS)` — 10% done +- `(50, DWNL_IN_PROGRESS)` — halfway +- `(100, DWNL_COMPLETED)` — finished successfully +- OR `(X, DWNL_ERROR)` — failed at X% + +#### Output + +| Return Value | Meaning | +|-------------|---------| +| `RDKFW_DWNL_SUCCESS` (0) | Download request sent. Callbacks will fire. | +| `RDKFW_DWNL_FAILED` (1) | Could not send request. No callbacks will fire. | + +#### Internal Steps + +``` +Step 1: Validate handle, fwdwnlreq, fwdwnlreq->firmwareName, callback +Step 2: Connect to D-Bus system bus +Step 3: Register callback in DwnlCallbackRegistry (slot: IDLE → ACTIVE) +Step 4: Fire-and-forget: DownloadFirmware(s handle, s firmwareName, s url, s type) + ├─ url defaults to "" if NULL + └─ type defaults to "" if NULL +Step 5: Unref connection, return SUCCESS +``` + +#### Key Difference from checkForUpdate() + +| Aspect | checkForUpdate | downloadFirmware | +|--------|---------------|-----------------| +| Callback fires | Once | Multiple times (every progress signal) | +| Registry slot lifecycle | PENDING → DISPATCHED → IDLE | ACTIVE → ACTIVE → ... → IDLE | +| Slot reset trigger | After single dispatch | Only on DWNL_COMPLETED or DWNL_ERROR | + +#### Thread Safety + +- Safe for concurrent calls (own registry with own mutex) +- Same handle calling `downloadFirmware()` twice overwrites the previous slot (prevents stale callbacks) + +#### Typical Caller Usage + +```c +void my_download_cb(int progress, DownloadStatus status) { + printf("Download: %d%% [%s]\n", progress, + status == DWNL_COMPLETED ? "DONE" : + status == DWNL_ERROR ? "ERROR" : "IN_PROGRESS"); + + if (status == DWNL_COMPLETED || status == DWNL_ERROR) { + // Signal main thread — download finished + pthread_mutex_lock(&dl_mutex); + dl_done = 1; + dl_status = status; + pthread_cond_signal(&dl_cond); + pthread_mutex_unlock(&dl_mutex); + } +} + +FwDwnlReq req = { + .firmwareName = "firmware_v2.bin", + .downloadUrl = NULL, // Let daemon use XConf URL + .TypeOfFirmware = "PCI" +}; + +DownloadResult rc = downloadFirmware(handle, &req, my_download_cb); +``` + +--- + +### 7.4 `updateFirmware()` + +```c +UpdateResult updateFirmware(FirmwareInterfaceHandle handle, + const FwUpdateReq *fwupdatereq, + UpdateCallback callback); +``` + +#### Purpose + +Initiates non-blocking firmware flashing. The daemon writes the previously downloaded firmware image to the device's storage partition. Progress is reported through repeated callback invocations. + +**WARNING**: This operation modifies device firmware. It is irreversible once the flash begins. + +#### Inputs + +| Parameter | Type | Constraints | +|-----------|------|-------------| +| `handle` | `FirmwareInterfaceHandle` | Non-NULL, non-empty | +| `fwupdatereq` | `const FwUpdateReq*` | Non-NULL; `firmwareName` and `TypeOfFirmware` required | +| `callback` | `UpdateCallback` | Non-NULL | + +```c +typedef struct { + const char *firmwareName; // REQUIRED: must match downloaded file + const char *TypeOfFirmware; // REQUIRED: "PCI", "PDRI", or "PERIPHERAL" + const char *LocationOfFirmware; // OPTIONAL: NULL → use /etc/device.properties default + bool rebootImmediately; // true → device reboots when flash completes +} FwUpdateReq; +``` + +#### Callback Signature + +```c +typedef void (*UpdateCallback)(int update_progress, UpdateStatus fwupdatestatus); +``` + +#### Output + +| Return Value | Meaning | +|-------------|---------| +| `RDKFW_UPDATE_SUCCESS` (0) | Flash request sent. Callbacks will fire. | +| `RDKFW_UPDATE_FAILED` (1) | Could not send request. No callbacks. | + +#### D-Bus Wire Format + +The library converts the `FwUpdateReq` struct to a D-Bus method call with signature `(sssss)`: +``` +s handle — "12345" +s firmwareName — "firmware_v2.bin" +s LocationOfFirmware — "/opt/CDL" (or "" if NULL) +s TypeOfFirmware — "PCI" +s rebootImmediately — "true" or "false" (string, not bool!) +``` + +Note: `rebootImmediately` is a `bool` in the struct but transmitted as a string because the daemon's D-Bus method expects string arguments. + +#### Thread Safety + +Same as `downloadFirmware()` — own registry, own mutex, safe for concurrent use. + +--- + +### 7.5 `unregisterProcess()` + +```c +void unregisterProcess(FirmwareInterfaceHandle handler); +``` + +#### Purpose + +Terminates the session with the daemon, stops the background thread, frees all library resources, and frees the handle memory. This is the mandatory last call. + +#### Inputs + +| Parameter | Type | Constraints | +|-----------|------|-------------| +| `handler` | `FirmwareInterfaceHandle` | May be NULL (no-op) | + +#### Output + +Returns `void`. This function always succeeds from the caller's perspective (best-effort cleanup). + +#### Internal Steps + +``` +Step 1: NULL check → if NULL, log info and return (no-op, safe) + +Step 2: Parse handle string → uint64 handler_id + ├─ Uses strtoull() with strict endptr validation + ├─ Rejects: "123abc", " 123", "", "abc", overflow + └─ On invalid: FWUPMGR_ERROR, free(handler), return + +Step 3: internal_system_deinit() + ├─ g_main_loop_quit() → background thread wakes up from g_main_loop_run() + ├─ pthread_join() → wait for background thread to exit cleanly + ├─ g_main_loop_unref() + g_main_context_unref() + ├─ internal_dwnl_system_deinit() → free download registry + ├─ internal_update_system_deinit() → free update registry + ├─ Free all remaining handle_key strings in check registry + └─ pthread_mutex_destroy() × 4 (bg_thread, registry, dwnl, update) + +Step 4: Create D-Bus proxy (best-effort — may fail if daemon is already gone) + +Step 5: Call UnregisterProcess(t handler_id) on daemon + ├─ Success: daemon removes ProcessInfo + └─ Failure: logged but ignored (daemon may have crashed/restarted) + +Step 6: free(handler) — the string is released regardless of D-Bus call outcome +``` + +#### Error Handling + +This function is **deliberately tolerant of errors**: + +| Error | Action | Cleanup Continues? | +|-------|--------|-------------------| +| NULL handle | No-op, return | N/A | +| Invalid handle string | Log error, free handle, return | Yes | +| D-Bus proxy creation fails | Log warning, continue | Yes — free(handle) still happens | +| UnregisterProcess D-Bus call fails | Log warning, continue | Yes — free(handle) still happens | +| Daemon already crashed | D-Bus call times out, logged | Yes — local cleanup still happens | + +#### Why `internal_system_deinit()` Before D-Bus Call? + +1. After unregistering, daemon stops sending signals → background thread is useless +2. `pthread_join()` returns immediately since `g_main_loop_quit()` unblocks the thread +3. If we sent D-Bus first and the daemon is slow, the background thread would sit idle waiting for signals that will never come + +#### Memory Ownership + +After `unregisterProcess()` returns: +- The handle pointer is **freed and invalid** — do not use it +- All background resources are released +- Library is back to "unlinked" state — `registerProcess()` can be called again if needed + +#### Typical Caller Usage + +```c +// Always call before exit, even on error paths +cleanup: + if (handle != NULL) { + unregisterProcess(handle); + handle = NULL; // Defensive: mark as invalid + } + return exit_code; +``` + +--- + +## 8. Internal Helper Modules + +### 8.1 Module: `rdkFwupdateMgr_async.c` — The Async Engine + +This is the core internal module. It owns: + +| Component | Purpose | +|-----------|---------| +| `g_registry` (CallbackRegistry) | Stores pending `checkForUpdate` callbacks | +| `g_dwnl_registry` (DwnlCallbackRegistry) | Stores active download callbacks | +| `g_update_registry` (UpdateCbRegistry) | Stores active update callbacks | +| `g_bg_thread` (BackgroundThread) | Holds thread handle, GMainLoop, connection, subscription IDs | + +#### Key Internal Functions + +| Function | Called By | Purpose | +|----------|-----------|---------| +| `internal_system_init()` | `registerProcess()` | Start everything: registries + thread | +| `internal_system_deinit()` | `unregisterProcess()` | Stop everything: thread + registries | +| `internal_register_callback()` | `checkForUpdate()` | Add callback to check registry | +| `internal_dwnl_register_callback()` | `downloadFirmware()` | Add callback to download registry | +| `internal_update_register_callback()` | `updateFirmware()` | Add callback to update registry | +| `background_thread_func()` | `pthread_create()` | Thread entry point: connect + subscribe + loop | +| `on_check_complete_signal()` | GLib (signal dispatch) | Parse signal → dispatch callbacks | +| `on_download_progress_signal()` | GLib (signal dispatch) | Parse signal → dispatch callbacks | +| `on_update_progress_signal()` | GLib (signal dispatch) | Parse signal → dispatch callbacks | +| `dispatch_all_pending()` | `on_check_complete_signal()` | Two-phase dispatch for check callbacks | +| `dispatch_all_dwnl_active()` | `on_download_progress_signal()` | Two-phase dispatch for download callbacks | +| `dispatch_all_update_active()` | `on_update_progress_signal()` | Two-phase dispatch for update callbacks | +| `parse_update_details()` | `dispatch_all_pending()` | Parse pipe-separated firmware details string | + +### 8.2 Module: `rdkFwupdateMgr_process.c` — Registration Logic + +Contains `registerProcess()` and `unregisterProcess()` plus two helpers: + +| Function | Purpose | +|----------|---------| +| `create_dbus_proxy()` | Create a GDBusProxy connected to daemon (used by both register and unregister) | +| `validate_process_name()` | NULL, empty, and length checks | +| `validate_lib_version()` | NULL and length checks | + +### 8.3 Module: `rdkFwupdateMgr_log.h` — Logging Macros + +Header-only. Defines `FWUPMGR_*` macros. See [Section 12](#12-logging-architecture) for full details. + +### 8.4 Module: `rdkFwupdateMgr_async_internal.h` — Internal Types + +Header for internal use only. Defines: +- All `typedef struct` types for registries and signal data +- All `internal_*` function declarations +- D-Bus constants (`DBUS_SERVICE_NAME`, etc.) +- Architecture ASCII diagrams in comments + +--- + +## 9. IPC Communication Model + +### 9.1 Transport: D-Bus System Bus + +| Property | Value | +|----------|-------| +| Bus type | System bus (`G_BUS_TYPE_SYSTEM`) | +| Well-known name | `org.rdkfwupdater.Service` | +| Object path | `/org/rdkfwupdater/Service` | +| Interface | `org.rdkfwupdater.Interface` | + +### 9.2 Connection Pattern: Ephemeral Per-Call + +``` +registerProcess(): [Connect] → [Call] → [Disconnect] (unique sender :1.140) +checkForUpdate(): [Connect] → [Call] → [Disconnect] (unique sender :1.141) +downloadFirmware(): [Connect] → [Call] → [Disconnect] (unique sender :1.142) +updateFirmware(): [Connect] → [Call] → [Disconnect] (unique sender :1.143) +unregisterProcess(): [Connect] → [Call] → [Disconnect] (unique sender :1.145) +``` + +**Background thread**: Has its OWN **persistent** connection for signal subscriptions. This connection lives for the entire library lifecycle. + +### 9.3 Method Calls (Client → Daemon) + +| Method | GVariant Signature | Direction | Blocking? | +|--------|-------------------|-----------|-----------| +| `RegisterProcess` | IN: `(ss)` OUT: `(t)` | Synchronous | Yes (5s timeout) | +| `UnregisterProcess` | IN: `(t)` OUT: `(b)` | Synchronous | Yes (5s timeout) | +| `CheckForUpdate` | IN: `(s)` OUT: none | Fire-and-forget | No | +| `DownloadFirmware` | IN: `(ssss)` OUT: none | Fire-and-forget | No | +| `UpdateFirmware` | IN: `(sssss)` OUT: none | Fire-and-forget | No | + +### 9.4 Signals (Daemon → Client) + +| Signal | GVariant Signature | Delivery | +|--------|-------------------|----------| +| `CheckForUpdateComplete` | `(tiissss)` | Once per check | +| `DownloadProgress` | `(tsuss)` | Repeated (per progress %) | +| `UpdateProgress` | `(tsiis)` | Repeated (per progress %) | + +### 9.5 Why Fire-and-Forget for Async Operations? + +For `checkForUpdate`, `downloadFirmware`, and `updateFirmware`: +- The actual work takes seconds to minutes +- Blocking the caller for that duration defeats the purpose +- The daemon acknowledges receipt implicitly by starting work +- Results come as signals (broadcast notifications) +- If the call fails at D-Bus level, `g_dbus_connection_call()` still returns successfully (message queued) — the failure manifests as no signal ever arriving + +--- + +## 10. Retry and Timeout Strategy + +### 10.1 Library-Side Timeouts + +| Operation | Timeout | Location | Behavior on Timeout | +|-----------|---------|----------|---------------------| +| `RegisterProcess` D-Bus call | 5000ms | `DBUS_TIMEOUT_MS` constant | Returns NULL with error log | +| `UnregisterProcess` D-Bus call | 5000ms | Same constant | Logs warning, continues cleanup | +| Fire-and-forget calls | 5000ms | Same constant | GLib queues message; timeout only applies to queueing | +| Background thread startup | 5000ms | `internal_system_init()` spin-wait | Continues anyway (first API call will fail) | + +### 10.2 Library-Side Retry Policy + +**The library does NOT retry.** This is a deliberate design decision: + +- Retry logic belongs in the **caller**, not the transport layer +- The caller knows the right retry interval and max attempts for their use case +- A monitoring daemon might retry every 60 seconds; a user-facing app might retry once after 5 seconds +- Silent retries inside the library would hide failures from the caller + +### 10.3 Caller-Side Timeout Guidance + +| Operation | Recommended Timeout | Rationale | +|-----------|-------------------|-----------| +| `checkForUpdate` callback | 120 seconds (2 min) | XConf HTTP query + network latency | +| `downloadFirmware` callback | 300 seconds (5 min) | Large firmware images over cellular | +| `updateFirmware` callback | 600 seconds (10 min) | Flash operations vary by storage type | + +The `example_app.c` uses exactly these values with `pthread_cond_timedwait()`. + +### 10.4 Callback Timeout Detection (Internal) + +Each registry entry stores a `registered_time` timestamp. The async engine could use this for internal timeout detection (sweeping stale entries). Currently, timeout detection is NOT actively enforced — the `TIMED_OUT` state exists in the state machine but no sweeper thread runs. + +**Recommendation for future work**: Add a periodic sweep (every 60s) in the background thread via `g_timeout_add()` to reset stale PENDING/ACTIVE entries that have exceeded `CALLBACK_TIMEOUT_SECONDS` (60s). + +--- + +## 11. Failure Scenarios and Recovery + +### 11.1 Daemon Not Running + +| Symptom | Detection | Recovery | +|---------|-----------|----------| +| `registerProcess()` returns NULL | D-Bus error: "The name org.rdkfwupdater.Service was not provided by any .service files" | Client logs error, retries, or exits | +| Fire-and-forget calls appear to succeed | No signal ever arrives | Client's condvar times out | + +### 11.2 Daemon Crashes Mid-Operation + +| Symptom | Detection | Recovery | +|---------|-----------|----------| +| No more signals arrive | Client's condvar times out | Client calls `unregisterProcess()` (best-effort), then re-registers | +| Background thread's D-Bus connection emits "closed" signal | Not currently handled | **Future work**: detect and propagate error to pending callbacks | + +### 11.3 D-Bus System Bus Restart + +| Symptom | Detection | Recovery | +|---------|-----------|----------| +| All D-Bus connections become invalid | Next API call fails at `g_bus_get_sync()` | Client calls `unregisterProcess()`, waits, tries `registerProcess()` again | + +### 11.4 Client Crashes Without Unregistering + +| Symptom | Detection | Recovery | +|---------|-----------|----------| +| Daemon holds stale ProcessInfo | Daemon watches D-Bus name owner changes (NameOwnerChanged signal) | Daemon auto-removes registration when client's bus name disappears | + +**Note**: Due to the per-call connection model, this detection is unreliable — each API call has a different bus name that immediately disappears after the call. The daemon relies on explicit `UnregisterProcess` or periodic cleanup. + +### 11.5 Registry Full (30 Slots) + +| Symptom | Detection | Recovery | +|---------|-----------|----------| +| `checkForUpdate`/`downloadFirmware`/`updateFirmware` returns FAIL | `internal_*_register_callback()` returns false | Client should wait for pending operations to complete, then retry | + +### 11.6 Signal Arrives Before Callback Registered + +| Cause | Prevention | Impact | +|-------|-----------|--------| +| Daemon responds extremely fast | Library registers callback BEFORE sending D-Bus call | Cannot happen with current ordering | +| Connect → Register → Send ordering eliminates this race | N/A | N/A | + +--- + +## 12. Logging Architecture + +### 12.1 Three-Module Design + +``` +┌──────────────────────────────────────────────────┐ +│ LOG OUTPUT │ +│ /opt/logs/rdkFwupdateMgr.log │ +├──────────────────────────────────────────────────┤ +│ [EXAMPLE] App-level messages │ ← Client app +│ [FWUPMGR] Library internal messages │ ← This library +│ [FWUPG] Daemon operational messages │ ← Daemon +└──────────────────────────────────────────────────┘ +``` + +| RDK_LOGGER Module | Macro Prefix | Source Files | Purpose | +|------------------|--------------|--------------|---------| +| `LOG.RDK.FWUPMGR` | `FWUPMGR_*` | `_process.c`, `_api.c`, `_async.c` | Library internals | +| `LOG.RDK.FWUPG` | `SWLOG_*` | Daemon sources | Daemon operations | +| `LOG.RDK.EXAMPLE` | `EXAMPLE_*` | `example_app.c` | Example client app | + +### 12.2 Log Levels Used + +| Level | Macro | Usage | +|-------|-------|-------| +| TRACE | `FWUPMGR_TRACE` | Not used currently (reserved for future verbose tracing) | +| DEBUG | `FWUPMGR_DEBUG` | Detailed internal state (registry operations, GVariant parsing) | +| INFO | `FWUPMGR_INFO` | Normal operation flow (API entry/exit, handle values, signal reception) | +| WARN | `FWUPMGR_WARN` | Recoverable issues (D-Bus call failed in unregister, daemon already gone) | +| ERROR | `FWUPMGR_ERROR` | Failures that cause API to return error code (validation, connection, registry full) | +| FATAL | `FWUPMGR_FATAL` | Not used currently (reserved for unrecoverable states) | + +### 12.3 Log Lifecycle Ownership + +```c +// CALLER'S responsibility — library never calls these: +log_init(); // Before any library API call +// ... use library ... +log_exit(); // After unregisterProcess() +``` + +**Rationale**: A process should call `log_init()` exactly once. If both the library and the app call it, double-initialization could corrupt state. + +### 12.4 Macro Implementation + +```c +// rdkFwupdateMgr_log.h (when RDK_LOGGER is defined): +#define FWUPMGR_LOG(level, module, format, ...) \ + RDK_LOG(level, module, format, ##__VA_ARGS__) + +#define FWUPMGR_INFO(format, ...) \ + FWUPMGR_LOG(RDK_LOG_INFO, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) + +// Client apps reuse the base macro with their own module: +#define MYAPP_INFO(format, ...) \ + FWUPMGR_LOG(RDK_LOG_INFO, "LOG.RDK.MYAPP", format, ##__VA_ARGS__) +``` + +### 12.5 Non-RDK_LOGGER Fallback + +For unit tests and development builds without RDK_LOGGER: +```c +#define FWUPMGR_LOG(level, module, FORMAT...) fprintf(stderr, "[%s] " FORMAT, module) +``` + +### 12.6 Build Dependencies + +```makefile +# In Makefile.am for any target that uses logging: +target_CFLAGS += -I${top_srcdir}/librdkFwupdateMgr/src # rdkFwupdateMgr_log.h +target_CFLAGS += -I${top_srcdir}/common_utilities/utils # rdkv_cdl_log_wrapper.h +target_LDADD += -lfwutils -lrdkloggers # Runtime implementations +``` + +--- + +## 13. Security Considerations + +### 13.1 D-Bus Policy Enforcement + +Access control is enforced at the D-Bus level via policy files: +```xml + + + + + +``` + +Only processes running as the configured user (typically root or a specific service account) can call methods or receive signals. + +### 13.2 Handler ID as Authorization Token + +The `handler_id` (e.g., `"12345"`) serves as the session authorization token: +- It's a daemon-generated 64-bit value (not guessable from outside) +- Required for all operations after registration +- The daemon validates that the handler_id exists before processing any request +- In this library model, sender-ID validation is not used (see Section 9.2 for why) + +**Limitation**: The handler_id is a sequential counter (not cryptographically random). In a multi-client environment, a malicious client could potentially guess another client's handler_id. For production hardening, consider using a random 128-bit token. + +### 13.3 Input Validation at Library Boundary + +All public API functions validate inputs before forwarding to D-Bus: +- NULL pointer checks on all parameters +- Empty string rejection +- Length limit enforcement (prevents buffer overflow in daemon's fixed-size buffers) +- Numeric parse validation in `unregisterProcess()` (strict `strtoull` with endptr check) + +### 13.4 No Credential Storage + +The library does not store, cache, or transmit any credentials. Authentication is entirely handled by D-Bus policy and daemon-side process tracking. + +### 13.5 Memory Safety + +- No `sprintf()` usage — all string formatting uses `snprintf()` with bounds +- All `strncpy()` usage explicitly null-terminates the destination +- Handle memory is freed exactly once (in `unregisterProcess()`) +- Registry entries are freed on slot reset — no dangling pointers +- GLib objects are unreffed on all code paths (including error paths) + +--- + +## 14. Performance Considerations + +### 14.1 Overhead Per API Call + +| Operation | Typical Latency | Bottleneck | +|-----------|----------------|------------| +| `registerProcess()` | 5-10ms | D-Bus round-trip | +| `checkForUpdate()` | <1ms (returns immediately) | D-Bus message queueing | +| `downloadFirmware()` | <1ms (returns immediately) | D-Bus message queueing | +| `updateFirmware()` | <1ms (returns immediately) | D-Bus message queueing | +| `unregisterProcess()` | 10-15ms | pthread_join + D-Bus round-trip | + +### 14.2 Memory Footprint + +| Component | Size | Notes | +|-----------|------|-------| +| Three registries (static arrays) | ~30 × 3 × (272 bytes per entry) ≈ 24 KB | Stack-like, no heap fragmentation | +| Background thread stack | 8 MB default (configurable) | Linux thread default | +| GMainLoop + GMainContext | ~2 KB | GLib internal | +| D-Bus connection (persistent) | ~4 KB | Background thread's connection | +| Per-call D-Bus proxy | ~2 KB | Freed immediately after call | + +**Total resident overhead**: ~35 KB (excluding thread stack, which is virtual memory only) + +### 14.3 Connection Overhead + +The per-call connection model has overhead: +- Each `g_bus_get_sync()` call performs a D-Bus handshake +- Each gets a unique sender name allocated by dbus-daemon +- Connection teardown releases the name + +For infrequent firmware operations (minutes/hours between calls), this overhead is negligible. If call frequency were high (>10/second), a persistent connection pool would be warranted. + +### 14.4 Signal Dispatch Efficiency + +The two-phase dispatch creates a stack-local snapshot array on every signal. With `MAX_PENDING_CALLBACKS=30` and ~272 bytes per snapshot entry, this is ~8KB of stack usage — well within typical 8MB thread stack limits. + +The linear scan of 30 entries is O(30) — negligible for this use case. A hash map would be over-engineering. + +--- + +## 15. Scalability for Multiple Clients + +### 15.1 Current Capacity + +| Resource | Limit | Constraint | +|----------|-------|------------| +| Concurrent registered processes | Limited by daemon's `registered_processes` hash table | Effectively unbounded (GHashTable) | +| Concurrent pending checkForUpdate callbacks | 30 | `MAX_PENDING_CALLBACKS` constant | +| Concurrent active downloads | 30 | Same constant for download registry | +| Concurrent active updates | 30 | Same constant for update registry | + +### 15.2 Multi-Client Signal Delivery + +D-Bus signals are **broadcast** — all connected clients receive them. The library's dispatch logic handles this: + +``` +Daemon emits CheckForUpdateComplete: + ├─ Client A's background thread receives it → dispatches to Client A's callback + ├─ Client B's background thread receives it → dispatches to Client B's callback + └─ Client C's background thread receives it → dispatches to Client C's callback +``` + +Each client has its own library instance (separate `.so` loaded into its process space), its own background thread, its own registries. They are completely independent. + +### 15.3 Daemon-Side Coalescing + +For download operations, the daemon implements **piggybacking**: if multiple clients request the same firmware file simultaneously, the daemon downloads it once and sends progress signals to all registered clients. The library dispatches these signals to all ACTIVE download callbacks transparently. + +### 15.4 Scaling Limitations + +- If more than 30 checkForUpdate calls are pending simultaneously (across the same process), the 31st will fail with `CHECK_FOR_UPDATE_FAIL`. This is unlikely in practice — 30 concurrent firmware checks from one process would be a design error. +- The background thread is single-threaded — if a callback takes a long time (e.g., client does heavy processing in the callback), other signals queue up in the GMainContext. Clients should keep callbacks short and signal their main thread for heavy work. + +--- + +## 16. Future Extensibility + +### 16.1 Potential New APIs + +| API | Purpose | Priority | +|-----|---------|----------| +| `cancelDownload(handle)` | Abort an in-progress download | High | +| `pauseDownload(handle)` / `resumeDownload(handle)` | Pause/resume for network bandwidth management | Medium | +| `getUpdateStatus(handle)` | Synchronous poll of current state (for apps that don't want callbacks) | Medium | +| `setUpdatePolicy(handle, policy)` | Configure auto-update behavior per client | Low | +| `subscribeToDeviceState(handle, cb)` | Get notified of device firmware state changes (not just self-initiated) | Low | + +### 16.2 HAL Integration (Planned) + +The `UpdateCallback` signature documentation notes: +> "The signature and behavior of this callback may change in future versions when HAL (Hardware Abstraction Layer) APIs become available." + +When HAL is integrated: +- More granular progress reporting (per-partition) +- Device-specific status codes +- Verification step callbacks (checksum validation) + +### 16.3 Persistent Connection Model (If Needed) + +If call frequency increases or the daemon implements per-connection state: +- Replace per-call `g_bus_get_sync()` with a connection pool +- Add reconnection logic on connection drop +- Add heartbeat/keepalive mechanism + +### 16.4 User-Data in Callbacks + +Current callback signatures don't include a `void *user_data` parameter (e.g., `UpdateEventCallback` takes only `const FwInfoData*`). Adding user_data would: +- Eliminate the need for global variables in client apps +- Allow multiple independent sessions in one process +- Be an ABI-breaking change (major version bump) + +### 16.5 Async/Await Style (C11 Atomics) + +For modern C codebases, consider offering a "future" API alongside callbacks: +```c +// Hypothetical future API: +FwCheckFuture *future = checkForUpdateAsync(handle); +// ... do other work ... +FwInfoData *result = awaitFwCheck(future, timeout_ms); // blocks until ready +freeFwCheckFuture(future); +``` + +This would be a convenience wrapper over the existing callback mechanism. + +--- + +## Appendix A: Complete D-Bus Interface Contract + +``` +Service: org.rdkfwupdater.Service +Path: /org/rdkfwupdater/Service +Interface: org.rdkfwupdater.Interface + +METHODS: + RegisterProcess(s processName, s libVersion) → (t handler_id) + UnregisterProcess(t handler_id) → (b success) + CheckForUpdate(s handle) + DownloadFirmware(s handle, s firmwareName, s downloadUrl, s typeOfFirmware) + UpdateFirmware(s handle, s firmwareName, s location, s type, s rebootImmediately) + +SIGNALS: + CheckForUpdateComplete(t handler_id, i result_code, i status_code, + s current_version, s available_version, + s update_details, s status_message) + + DownloadProgress(t handler_id, s firmware_name, u progress_percent, + s status_string, s message) + + UpdateProgress(t handler_id, s firmware_name, i progress_percent, + i status_code, s message) +``` + +--- + +## Appendix B: End-to-End Sequence Diagram + +``` + Client App (main) librdkFwupdateMgr BG Thread Daemon + ───────────────── ───────────────── ───────── ────── + │ │ │ │ + │ registerProcess() │ │ │ + │──────────────────────────►│ │ │ + │ │── D-Bus: Register ───────────────────────►│ + │ │◄──── handler_id ─────────────────────────┤ + │ │── internal_system_init() │ + │ │ └─ pthread_create() ──►│ │ + │ │ │─ subscribe ──►│ + │ │ │ signals │ + │ │◄── running=true ─────────┤ │ + │◄── handle "12345" ────────┤ │ │ + │ │ │ │ + │ checkForUpdate(h, cb) │ │ │ + │──────────────────────────►│ │ │ + │ │── register cb in reg. │ │ + │ │── D-Bus: CheckForUpdate ─────────────────►│ + │◄── SUCCESS ───────────────┤ │ │ + │ │ │ │ + │ [waiting on condvar] │ │ XConf... │ + │ │ │ │ + │ │ │◄── signal ────┤ + │ │ │ │ + │ │ on_check_complete_signal() │ + │ │ dispatch_all_pending() │ + │ │ │ │ + │◄──────────────────────────│────── cb(&fwinfo) ◄┘ │ + │ [condvar signaled] │ │ + │ │ │ + │ downloadFirmware(h,r,cb) │ │ + │──────────────────────────►│ │ + │ │── register dl_cb │ + │ │── D-Bus: DownloadFirmware ───────────────►│ + │◄── SUCCESS ───────────────┤ │ │ + │ │ │◄── 25% ───────┤ + │◄──── dl_cb(25,INPROG) ────│──────────────────────────┤ │ + │ │ │◄── 100% ──────┤ + │◄──── dl_cb(100,DONE) ─────│──────────────────────────┤ │ + │ │ │ + │ unregisterProcess(h) │ │ + │──────────────────────────►│ │ + │ │── internal_system_deinit() │ + │ │ └─ quit loop ──────────►│ exits │ + │ │ └─ join ◄──────────────┘ │ + │ │── D-Bus: Unregister ─────────────────────►│ + │ │── free(handle) │ + │◄── return ────────────────┤ │ + │ │ │ +``` + +--- + +## Appendix C: Build and Integration + +### Library Build (Autotools) + +```makefile +# librdkFwupdateMgr/Makefile.am +lib_LTLIBRARIES = librdkFwupdateMgr.la + +librdkFwupdateMgr_la_SOURCES = \ + src/rdkFwupdateMgr_process.c \ + src/rdkFwupdateMgr_api.c \ + src/rdkFwupdateMgr_async.c + +librdkFwupdateMgr_la_CFLAGS = \ + -I$(top_srcdir)/librdkFwupdateMgr/include \ + -I$(top_srcdir)/librdkFwupdateMgr/src \ + -I$(top_srcdir)/common_utilities/utils \ + $(GIO_CFLAGS) $(GLIB_CFLAGS) + +librdkFwupdateMgr_la_LIBADD = \ + $(GIO_LIBS) $(GLIB_LIBS) -lpthread + +# Installed public header +librdkFwupdateMgr_includedir = $(includedir)/rdkFwupdateMgr +librdkFwupdateMgr_include_HEADERS = include/rdkFwupdateMgr_client.h +``` + +### Client Linking + +```makefile +my_app_LDADD = -lrdkFwupdateMgr -lfwutils -lrdkloggers $(GIO_LIBS) +my_app_CFLAGS = -I$(includedir)/rdkFwupdateMgr +``` + +### Minimum Client Code + +```c +#include "rdkFwupdateMgr_client.h" +#include "rdkv_cdl_log_wrapper.h" + +int main(void) { + log_init(); + + FirmwareInterfaceHandle h = registerProcess("MyApp", LIB_VERSION); + if (!h) return 1; + + // ... use APIs ... + + unregisterProcess(h); + log_exit(); + return 0; +} +``` + +--- + +*End of Document* diff --git a/librdkFwupdateMgr/docs/VISUAL_DESIGN_DOCUMENT.md b/librdkFwupdateMgr/docs/VISUAL_DESIGN_DOCUMENT.md new file mode 100755 index 00000000..a08461e8 --- /dev/null +++ b/librdkFwupdateMgr/docs/VISUAL_DESIGN_DOCUMENT.md @@ -0,0 +1,1342 @@ +# librdkFwupdateMgr — Visual Engineering Documentation + +> **Document Version**: 1.0 +> **Date**: April 28, 2026 +> **Classification**: Internal Engineering — Pull Request Review +> **Component**: `librdkFwupdateMgr` (shared library) +> **Companion**: See [DESIGN_DOCUMENT.md](DESIGN_DOCUMENT.md) for full prose reference + +--- + +## Color Legend + +| Color | Meaning | Used For | +|-------|---------|----------| +| 🔵 Blue | Public API | Exported functions, client-visible interfaces | +| 🟢 Green | Success | Successful returns, normal completion paths | +| 🔴 Red | Error | Failures, error returns, exception paths | +| 🟡 Yellow | Validation | Input checks, parameter validation gates | +| 🟣 Purple | Logging | Log emission points, log module boundaries | +| ⬜ Gray | Internal Helpers | Private functions, internal state management | + +--- + +## Table of Contents + +1. [High-Level Architecture](#1-high-level-architecture) +2. [API Flowcharts](#2-api-flowcharts) + - 2.1 [registerProcess()](#21-registerprocess) + - 2.2 [checkForUpdate()](#22-checkforupdate) + - 2.3 [downloadFirmware()](#23-downloadfirmware) + - 2.4 [updateFirmware()](#24-updatefirmware) + - 2.5 [unregisterProcess()](#25-unregisterprocess) +3. [Sequence Diagrams](#3-sequence-diagrams) + - 3.1 [Complete Firmware Update Flow](#31-complete-firmware-update-flow) + - 3.2 [Daemon Unavailable + Retry](#32-daemon-unavailable--retry) + - 3.3 [Callback Registration & Delivery](#33-callback-registration--delivery) + - 3.4 [Timeout Recovery](#34-timeout-recovery) +4. [Thread Safety Diagram](#4-thread-safety-diagram) +5. [Memory Ownership Diagram](#5-memory-ownership-diagram) +6. [Logging Pipeline Diagram](#6-logging-pipeline-diagram) + +--- + +## 1. High-Level Architecture + +```mermaid +graph TB + subgraph CLIENT["🔵 Client Applications"] + direction LR + P1["example_plugin"] + P2["TR-069 Agent"] + P3["WebUI Service"] + end + + subgraph LIB["🔵 librdkFwupdateMgr.so"] + direction TB + API["Public API Layer
registerProcess · checkForUpdate
downloadFirmware · updateFirmware
unregisterProcess"] + VAL["🟡 Validation Gate
NULL checks · length limits
handle verification"] + REG["⬜ Callback Registries
g_registry (check)
g_dwnl_registry (download)
g_update_registry (update)"] + BGT["⬜ Background Thread
GMainLoop · signal handlers
two-phase dispatch"] + LOG_LIB["🟣 LOG.RDK.FWUPMGR"] + + API --> VAL + VAL --> REG + REG --> BGT + API -.-> LOG_LIB + VAL -.-> LOG_LIB + REG -.-> LOG_LIB + BGT -.-> LOG_LIB + end + + subgraph IPC["⬜ IPC Layer — D-Bus System Bus"] + direction LR + METHODS["Method Calls
RegisterProcess · UnregisterProcess
CheckForUpdate · DownloadFirmware
UpdateFirmware"] + SIGNALS["Signals (Broadcast)
CheckForUpdateComplete
DownloadProgress
UpdateProgress"] + end + + subgraph DAEMON["⬜ rdkFwupdateMgr Daemon"] + direction TB + DBUS_SRV["D-Bus Server
Method handler · Signal emitter"] + PROC_MGR["Process Manager
handler_id tracking
registration table"] + LOG_DMN["🟣 LOG.RDK.FWUPG"] + + DBUS_SRV --> PROC_MGR + DBUS_SRV -.-> LOG_DMN + PROC_MGR -.-> LOG_DMN + end + + subgraph FW["⬜ Firmware Subsystem"] + direction LR + XCONF["XConf Server
(cloud)"] + CDN["CDN
(firmware images)"] + HAL["Device HAL
(flash storage)"] + REBOOT["Reboot Manager"] + end + + P1 --> API + P2 --> API + P3 --> API + + API -- "ephemeral connection
per API call" --> METHODS + BGT -- "persistent connection
signal subscription" --> SIGNALS + + METHODS --> DBUS_SRV + DBUS_SRV --> SIGNALS + + PROC_MGR --> XCONF + PROC_MGR --> CDN + PROC_MGR --> HAL + PROC_MGR --> REBOOT + + style CLIENT fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style LIB fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style IPC fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style DAEMON fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style FW fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + + style API fill:#3b82f6,stroke:#1d4ed8,color:#fff + style VAL fill:#eab308,stroke:#a16207,color:#fff + style REG fill:#9ca3af,stroke:#4b5563,color:#fff + style BGT fill:#9ca3af,stroke:#4b5563,color:#fff + style LOG_LIB fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_DMN fill:#a855f7,stroke:#7e22ce,color:#fff + style DBUS_SRV fill:#9ca3af,stroke:#4b5563,color:#fff + style PROC_MGR fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +### Layer Responsibilities Summary + +```mermaid +graph LR + subgraph LAYERS["System Layers"] + direction TB + L1["🔵 Client Layer
Calls 5 APIs · implements 3 callbacks
owns condvar sync · owns log lifecycle"] + L2["🔵 Library Layer
Validates · Transports · Dispatches
owns handle · owns BG thread"] + L3["⬜ IPC Layer
D-Bus system bus
methods ↓ · signals ↑"] + L4["⬜ Daemon Layer
Orchestrates firmware ops
owns process table · emits signals"] + L5["⬜ Firmware Subsystem
XConf · CDN · HAL · Reboot"] + + L1 --> L2 --> L3 --> L4 --> L5 + end + + style L1 fill:#3b82f6,stroke:#1d4ed8,color:#fff + style L2 fill:#3b82f6,stroke:#1d4ed8,color:#fff + style L3 fill:#d1d5db,stroke:#6b7280,color:#374151 + style L4 fill:#d1d5db,stroke:#6b7280,color:#374151 + style L5 fill:#d1d5db,stroke:#6b7280,color:#374151 +``` + +--- + +## 2. API Flowcharts + +### 2.1 `registerProcess()` + +```mermaid +flowchart TD + START(["🔵 registerProcess(processName, libVersion)"]) + START --> LOG_ENTRY["🟣 FWUPMGR_INFO: Entry with processName"] + + LOG_ENTRY --> V1{"🟡 processName == NULL?"} + V1 -- Yes --> ERR1["🔴 FWUPMGR_ERROR: NULL processName"] + ERR1 --> RET_NULL1(["🔴 return NULL"]) + V1 -- No --> V2{"🟡 processName empty?"} + + V2 -- Yes --> ERR2["🔴 FWUPMGR_ERROR: empty processName"] + ERR2 --> RET_NULL2(["🔴 return NULL"]) + V2 -- No --> V3{"🟡 strlen > 256?"} + + V3 -- Yes --> ERR3["🔴 FWUPMGR_ERROR: name too long"] + ERR3 --> RET_NULL3(["🔴 return NULL"]) + V3 -- No --> V4{"🟡 libVersion == NULL?"} + + V4 -- Yes --> ERR4["🔴 FWUPMGR_ERROR: NULL libVersion"] + ERR4 --> RET_NULL4(["🔴 return NULL"]) + V4 -- No --> V5{"🟡 strlen(libVersion) > 64?"} + + V5 -- Yes --> ERR5["🔴 FWUPMGR_ERROR: version too long"] + ERR5 --> RET_NULL5(["🔴 return NULL"]) + V5 -- No --> DBUS_CONN["⬜ g_bus_get_sync(SYSTEM)"] + + DBUS_CONN --> CONN_OK{"Connection OK?"} + CONN_OK -- No --> ERR6["🔴 FWUPMGR_ERROR: D-Bus connect failed"] + ERR6 --> RET_NULL6(["🔴 return NULL"]) + CONN_OK -- Yes --> PROXY["⬜ g_dbus_proxy_new_sync()"] + + PROXY --> PROXY_OK{"Proxy OK?"} + PROXY_OK -- No --> ERR7["🔴 FWUPMGR_ERROR: proxy creation failed"] + ERR7 --> UNREF_CONN1["⬜ g_object_unref(connection)"] + UNREF_CONN1 --> RET_NULL7(["🔴 return NULL"]) + + PROXY_OK -- Yes --> CALL["⬜ g_dbus_proxy_call_sync
RegisterProcess(ss) → (t)
timeout: 5000ms"] + + CALL --> CALL_OK{"D-Bus reply OK?"} + CALL_OK -- No --> ERR8["🔴 FWUPMGR_ERROR: daemon rejected / timeout"] + ERR8 --> UNREF_PROXY1["⬜ g_object_unref(proxy)"] + UNREF_PROXY1 --> RET_NULL8(["🔴 return NULL"]) + + CALL_OK -- Yes --> EXTRACT["⬜ Extract handler_id (uint64)
from GVariant reply"] + EXTRACT --> UNREF_RES["⬜ Unref result + proxy"] + UNREF_RES --> MALLOC["⬜ malloc(32) for handle string"] + + MALLOC --> MALLOC_OK{"malloc OK?"} + MALLOC_OK -- No --> ROLLBACK["🔴 Best-effort: call UnregisterProcess
to undo daemon-side registration"] + ROLLBACK --> RET_NULL9(["🔴 return NULL"]) + + MALLOC_OK -- Yes --> SNPRINTF["⬜ snprintf(handle, 32, handler_id)"] + SNPRINTF --> INIT["⬜ internal_system_init()"] + + INIT --> INIT_MUT["⬜ Initialize 3 mutexes"] + INIT_MUT --> INIT_REG["⬜ Zero 3 callback registries"] + INIT_REG --> INIT_CTX["⬜ Create GMainContext + GMainLoop"] + INIT_CTX --> INIT_THR["⬜ pthread_create(bg_thread)"] + INIT_THR --> INIT_WAIT["⬜ Spin-wait: bg_thread running? (max 5s)"] + + INIT_WAIT --> LOG_EXIT["🟣 FWUPMGR_INFO: Registered, handle=X"] + LOG_EXIT --> RET_OK(["🟢 return handle"]) + + style START fill:#3b82f6,stroke:#1d4ed8,color:#fff + style RET_OK fill:#22c55e,stroke:#15803d,color:#fff + style RET_NULL1 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL2 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL3 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL4 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL5 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL6 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL7 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL8 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_NULL9 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR1 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR2 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR3 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR4 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR5 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR6 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR7 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR8 fill:#ef4444,stroke:#b91c1c,color:#fff + style ROLLBACK fill:#ef4444,stroke:#b91c1c,color:#fff + style V1 fill:#eab308,stroke:#a16207,color:#fff + style V2 fill:#eab308,stroke:#a16207,color:#fff + style V3 fill:#eab308,stroke:#a16207,color:#fff + style V4 fill:#eab308,stroke:#a16207,color:#fff + style V5 fill:#eab308,stroke:#a16207,color:#fff + style LOG_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_EXIT fill:#a855f7,stroke:#7e22ce,color:#fff + style DBUS_CONN fill:#9ca3af,stroke:#4b5563,color:#fff + style PROXY fill:#9ca3af,stroke:#4b5563,color:#fff + style CALL fill:#9ca3af,stroke:#4b5563,color:#fff + style EXTRACT fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_RES fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_CONN1 fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_PROXY1 fill:#9ca3af,stroke:#4b5563,color:#fff + style MALLOC fill:#9ca3af,stroke:#4b5563,color:#fff + style SNPRINTF fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT_MUT fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT_REG fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT_CTX fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT_THR fill:#9ca3af,stroke:#4b5563,color:#fff + style INIT_WAIT fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +### 2.2 `checkForUpdate()` + +```mermaid +flowchart TD + START(["🔵 checkForUpdate(handle, callback)"]) + START --> LOG_ENTRY["🟣 FWUPMGR_INFO: checkForUpdate entry"] + + LOG_ENTRY --> V1{"🟡 handle == NULL
or empty?"} + V1 -- Yes --> ERR1["🔴 FWUPMGR_ERROR: invalid handle"] + ERR1 --> RET_FAIL1(["🔴 return CHECK_FOR_UPDATE_FAIL"]) + V1 -- No --> V2{"🟡 callback == NULL?"} + + V2 -- Yes --> ERR2["🔴 FWUPMGR_ERROR: NULL callback"] + ERR2 --> RET_FAIL2(["🔴 return CHECK_FOR_UPDATE_FAIL"]) + V2 -- No --> DBUS["⬜ g_bus_get_sync(SYSTEM)"] + + DBUS --> DBUS_OK{"Connection OK?"} + DBUS_OK -- No --> ERR3["🔴 FWUPMGR_ERROR: D-Bus connect failed"] + ERR3 --> RET_FAIL3(["🔴 return CHECK_FOR_UPDATE_FAIL"]) + + DBUS_OK -- Yes --> LOCK["⬜ pthread_mutex_lock(registry)"] + LOCK --> REG{"⬜ Find IDLE slot
in g_registry
(max 30)"} + REG -- Full --> ERR4["🔴 FWUPMGR_ERROR: registry full"] + ERR4 --> UNLOCK_ERR["⬜ pthread_mutex_unlock"] + UNLOCK_ERR --> UNREF_ERR["⬜ g_object_unref(conn)"] + UNREF_ERR --> RET_FAIL4(["🔴 return CHECK_FOR_UPDATE_FAIL"]) + + REG -- Found --> STORE["⬜ Store callback + handle_key
slot state: IDLE → PENDING"] + STORE --> UNLOCK["⬜ pthread_mutex_unlock"] + + UNLOCK --> FIRE["⬜ g_dbus_connection_call
CheckForUpdate(s handle)
fire-and-forget, timeout=5000ms"] + + FIRE --> UNREF["⬜ g_object_unref(connection)"] + UNREF --> LOG_EXIT["🟣 FWUPMGR_INFO: request sent"] + LOG_EXIT --> RET_OK(["🟢 return CHECK_FOR_UPDATE_SUCCESS"]) + + style START fill:#3b82f6,stroke:#1d4ed8,color:#fff + style RET_OK fill:#22c55e,stroke:#15803d,color:#fff + style RET_FAIL1 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL2 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL3 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL4 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR1 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR2 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR3 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR4 fill:#ef4444,stroke:#b91c1c,color:#fff + style V1 fill:#eab308,stroke:#a16207,color:#fff + style V2 fill:#eab308,stroke:#a16207,color:#fff + style LOG_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_EXIT fill:#a855f7,stroke:#7e22ce,color:#fff + style DBUS fill:#9ca3af,stroke:#4b5563,color:#fff + style LOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style REG fill:#9ca3af,stroke:#4b5563,color:#fff + style STORE fill:#9ca3af,stroke:#4b5563,color:#fff + style UNLOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style UNLOCK_ERR fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_ERR fill:#9ca3af,stroke:#4b5563,color:#fff + style FIRE fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +### 2.3 `downloadFirmware()` + +```mermaid +flowchart TD + START(["🔵 downloadFirmware(handle, fwdwnlreq, callback)"]) + START --> LOG_ENTRY["🟣 FWUPMGR_INFO: downloadFirmware entry"] + + LOG_ENTRY --> V1{"🟡 handle == NULL
or empty?"} + V1 -- Yes --> ERR1["🔴 FWUPMGR_ERROR: invalid handle"] --> RET_FAIL1(["🔴 return RDKFW_DWNL_FAILED"]) + V1 -- No --> V2{"🟡 fwdwnlreq == NULL?"} + + V2 -- Yes --> ERR2["🔴 FWUPMGR_ERROR: NULL request"] --> RET_FAIL2(["🔴 return RDKFW_DWNL_FAILED"]) + V2 -- No --> V3{"🟡 firmwareName
== NULL or empty?"} + + V3 -- Yes --> ERR3["🔴 FWUPMGR_ERROR: no firmware name"] --> RET_FAIL3(["🔴 return RDKFW_DWNL_FAILED"]) + V3 -- No --> V4{"🟡 callback == NULL?"} + + V4 -- Yes --> ERR4["🔴 FWUPMGR_ERROR: NULL callback"] --> RET_FAIL4(["🔴 return RDKFW_DWNL_FAILED"]) + V4 -- No --> DBUS["⬜ g_bus_get_sync(SYSTEM)"] + + DBUS --> DBUS_OK{"Connection OK?"} + DBUS_OK -- No --> ERR5["🔴 FWUPMGR_ERROR: D-Bus failed"] --> RET_FAIL5(["🔴 return RDKFW_DWNL_FAILED"]) + + DBUS_OK -- Yes --> LOCK["⬜ pthread_mutex_lock(dwnl_registry)"] + LOCK --> REG["⬜ Find/overwrite slot
in g_dwnl_registry
state: IDLE → ACTIVE"] + REG --> UNLOCK["⬜ pthread_mutex_unlock"] + + UNLOCK --> DEFAULT["⬜ Default NULL fields to empty string
url = fwdwnlreq→downloadUrl ?? ''
type = fwdwnlreq→TypeOfFirmware ?? ''"] + + DEFAULT --> FIRE["⬜ g_dbus_connection_call
DownloadFirmware(s handle, s name, s url, s type)
fire-and-forget"] + + FIRE --> UNREF["⬜ g_object_unref(connection)"] + UNREF --> LOG_EXIT["🟣 FWUPMGR_INFO: download request sent"] + LOG_EXIT --> RET_OK(["🟢 return RDKFW_DWNL_SUCCESS"]) + + style START fill:#3b82f6,stroke:#1d4ed8,color:#fff + style RET_OK fill:#22c55e,stroke:#15803d,color:#fff + style RET_FAIL1 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL2 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL3 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL4 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_FAIL5 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR1 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR2 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR3 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR4 fill:#ef4444,stroke:#b91c1c,color:#fff + style ERR5 fill:#ef4444,stroke:#b91c1c,color:#fff + style V1 fill:#eab308,stroke:#a16207,color:#fff + style V2 fill:#eab308,stroke:#a16207,color:#fff + style V3 fill:#eab308,stroke:#a16207,color:#fff + style V4 fill:#eab308,stroke:#a16207,color:#fff + style LOG_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_EXIT fill:#a855f7,stroke:#7e22ce,color:#fff + style DBUS fill:#9ca3af,stroke:#4b5563,color:#fff + style LOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style REG fill:#9ca3af,stroke:#4b5563,color:#fff + style UNLOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style DEFAULT fill:#9ca3af,stroke:#4b5563,color:#fff + style FIRE fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +### 2.4 `updateFirmware()` + +```mermaid +flowchart TD + START(["🔵 updateFirmware(handle, fwupdatereq, callback)"]) + START --> LOG_ENTRY["🟣 FWUPMGR_INFO: updateFirmware entry"] + + LOG_ENTRY --> V1{"🟡 handle == NULL
or empty?"} + V1 -- Yes --> E1["🔴 FWUPMGR_ERROR: invalid handle"] --> F1(["🔴 return RDKFW_UPDATE_FAILED"]) + V1 -- No --> V2{"🟡 fwupdatereq == NULL?"} + + V2 -- Yes --> E2["🔴 FWUPMGR_ERROR: NULL request"] --> F2(["🔴 return RDKFW_UPDATE_FAILED"]) + V2 -- No --> V3{"🟡 firmwareName
NULL or empty?"} + + V3 -- Yes --> E3["🔴 FWUPMGR_ERROR: missing name"] --> F3(["🔴 return RDKFW_UPDATE_FAILED"]) + V3 -- No --> V4{"🟡 TypeOfFirmware
NULL or empty?"} + + V4 -- Yes --> E4["🔴 FWUPMGR_ERROR: missing type"] --> F4(["🔴 return RDKFW_UPDATE_FAILED"]) + V4 -- No --> V5{"🟡 callback == NULL?"} + + V5 -- Yes --> E5["🔴 FWUPMGR_ERROR: NULL callback"] --> F5(["🔴 return RDKFW_UPDATE_FAILED"]) + V5 -- No --> DBUS["⬜ g_bus_get_sync(SYSTEM)"] + + DBUS --> OK{"Connection OK?"} + OK -- No --> E6["🔴 FWUPMGR_ERROR: D-Bus failed"] --> F6(["🔴 return RDKFW_UPDATE_FAILED"]) + + OK -- Yes --> LOCK["⬜ pthread_mutex_lock(update_registry)"] + LOCK --> REG["⬜ Register callback
state: IDLE → ACTIVE"] + REG --> UNLOCK["⬜ pthread_mutex_unlock"] + + UNLOCK --> CONV["⬜ Convert rebootImmediately
bool → string: 'true'/'false'
Default location to '' if NULL"] + + CONV --> FIRE["⬜ g_dbus_connection_call
UpdateFirmware(s handle, s name,
s location, s type, s reboot)
fire-and-forget"] + + FIRE --> UNREF["⬜ g_object_unref(connection)"] + UNREF --> LOG_EXIT["🟣 FWUPMGR_INFO: update request sent"] + LOG_EXIT --> RET_OK(["🟢 return RDKFW_UPDATE_SUCCESS"]) + + style START fill:#3b82f6,stroke:#1d4ed8,color:#fff + style RET_OK fill:#22c55e,stroke:#15803d,color:#fff + style F1 fill:#ef4444,stroke:#b91c1c,color:#fff + style F2 fill:#ef4444,stroke:#b91c1c,color:#fff + style F3 fill:#ef4444,stroke:#b91c1c,color:#fff + style F4 fill:#ef4444,stroke:#b91c1c,color:#fff + style F5 fill:#ef4444,stroke:#b91c1c,color:#fff + style F6 fill:#ef4444,stroke:#b91c1c,color:#fff + style E1 fill:#ef4444,stroke:#b91c1c,color:#fff + style E2 fill:#ef4444,stroke:#b91c1c,color:#fff + style E3 fill:#ef4444,stroke:#b91c1c,color:#fff + style E4 fill:#ef4444,stroke:#b91c1c,color:#fff + style E5 fill:#ef4444,stroke:#b91c1c,color:#fff + style E6 fill:#ef4444,stroke:#b91c1c,color:#fff + style V1 fill:#eab308,stroke:#a16207,color:#fff + style V2 fill:#eab308,stroke:#a16207,color:#fff + style V3 fill:#eab308,stroke:#a16207,color:#fff + style V4 fill:#eab308,stroke:#a16207,color:#fff + style V5 fill:#eab308,stroke:#a16207,color:#fff + style LOG_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_EXIT fill:#a855f7,stroke:#7e22ce,color:#fff + style DBUS fill:#9ca3af,stroke:#4b5563,color:#fff + style LOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style REG fill:#9ca3af,stroke:#4b5563,color:#fff + style UNLOCK fill:#9ca3af,stroke:#4b5563,color:#fff + style CONV fill:#9ca3af,stroke:#4b5563,color:#fff + style FIRE fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +### 2.5 `unregisterProcess()` + +```mermaid +flowchart TD + START(["🔵 unregisterProcess(handle)"]) + START --> V1{"🟡 handle == NULL?"} + + V1 -- Yes --> LOG_NULL["🟣 FWUPMGR_INFO: NULL handle, no-op"] + LOG_NULL --> RET_VOID1(["🟢 return (void)"]) + + V1 -- No --> PARSE["⬜ strtoull(handle) → handler_id"] + PARSE --> PARSE_OK{"🟡 Parse valid?
strict endptr check"} + + PARSE_OK -- No --> ERR1["🔴 FWUPMGR_ERROR: invalid handle format"] + ERR1 --> FREE_HANDLE_ERR["⬜ free(handle)"] + FREE_HANDLE_ERR --> RET_VOID2(["🔴 return (void)"]) + + PARSE_OK -- Yes --> LOG_DEINIT["🟣 FWUPMGR_INFO: deinit starting"] + LOG_DEINIT --> DEINIT["⬜ internal_system_deinit()"] + + DEINIT --> QUIT["⬜ g_main_loop_quit()"] + QUIT --> JOIN["⬜ pthread_join(bg_thread)"] + JOIN --> UNREF_LOOP["⬜ g_main_loop_unref()
g_main_context_unref()"] + UNREF_LOOP --> FREE_DWNL["⬜ internal_dwnl_system_deinit()"] + FREE_DWNL --> FREE_UPD["⬜ internal_update_system_deinit()"] + FREE_UPD --> FREE_REG["⬜ Free check registry handle_keys"] + FREE_REG --> DESTROY_MTX["⬜ pthread_mutex_destroy() × 4"] + + DESTROY_MTX --> PROXY["⬜ Create D-Bus proxy
(best-effort)"] + PROXY --> PROXY_OK{"Proxy OK?"} + + PROXY_OK -- No --> LOG_WARN["🟣 FWUPMGR_WARN: daemon unreachable"] + LOG_WARN --> FREE_HANDLE2["⬜ free(handle)"] + FREE_HANDLE2 --> RET_VOID3(["🟢 return (void)"]) + + PROXY_OK -- Yes --> CALL["⬜ g_dbus_proxy_call_sync
UnregisterProcess(t handler_id)"] + CALL --> CALL_OK{"D-Bus OK?"} + + CALL_OK -- No --> LOG_WARN2["🟣 FWUPMGR_WARN: unregister call failed"] + LOG_WARN2 --> UNREF_P2["⬜ g_object_unref(proxy)"] + UNREF_P2 --> FREE_HANDLE3["⬜ free(handle)"] + FREE_HANDLE3 --> RET_VOID4(["🟢 return (void)"]) + + CALL_OK -- Yes --> UNREF_ALL["⬜ g_object_unref(result + proxy)"] + UNREF_ALL --> LOG_OK["🟣 FWUPMGR_INFO: unregistered OK"] + LOG_OK --> FREE_HANDLE4["⬜ free(handle)"] + FREE_HANDLE4 --> RET_VOID5(["🟢 return (void)"]) + + style START fill:#3b82f6,stroke:#1d4ed8,color:#fff + style RET_VOID1 fill:#22c55e,stroke:#15803d,color:#fff + style RET_VOID2 fill:#ef4444,stroke:#b91c1c,color:#fff + style RET_VOID3 fill:#22c55e,stroke:#15803d,color:#fff + style RET_VOID4 fill:#22c55e,stroke:#15803d,color:#fff + style RET_VOID5 fill:#22c55e,stroke:#15803d,color:#fff + style ERR1 fill:#ef4444,stroke:#b91c1c,color:#fff + style V1 fill:#eab308,stroke:#a16207,color:#fff + style PARSE_OK fill:#eab308,stroke:#a16207,color:#fff + style LOG_NULL fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_DEINIT fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_WARN fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_WARN2 fill:#a855f7,stroke:#7e22ce,color:#fff + style LOG_OK fill:#a855f7,stroke:#7e22ce,color:#fff + style PARSE fill:#9ca3af,stroke:#4b5563,color:#fff + style DEINIT fill:#9ca3af,stroke:#4b5563,color:#fff + style QUIT fill:#9ca3af,stroke:#4b5563,color:#fff + style JOIN fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_LOOP fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_DWNL fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_UPD fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_REG fill:#9ca3af,stroke:#4b5563,color:#fff + style DESTROY_MTX fill:#9ca3af,stroke:#4b5563,color:#fff + style PROXY fill:#9ca3af,stroke:#4b5563,color:#fff + style CALL fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_ALL fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_HANDLE_ERR fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_HANDLE2 fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_HANDLE3 fill:#9ca3af,stroke:#4b5563,color:#fff + style FREE_HANDLE4 fill:#9ca3af,stroke:#4b5563,color:#fff + style UNREF_P2 fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +## 3. Sequence Diagrams + +### 3.1 Complete Firmware Update Flow + +```mermaid +sequenceDiagram + autonumber + participant App as 🔵 Client App + participant Lib as 🔵 Library API + participant BG as ⬜ BG Thread + participant Bus as ⬜ D-Bus + participant Dmn as ⬜ Daemon + participant XConf as ⬜ XConf Cloud + participant CDN as ⬜ CDN + participant HAL as ⬜ Device HAL + + Note over App,HAL: Phase 1 — Registration + + App->>+Lib: registerProcess("MyPlugin", "1.0") + Lib-->>Lib: 🟡 validate inputs + Lib->>+Bus: RegisterProcess(ss) + Bus->>+Dmn: RegisterProcess + Dmn-->>Dmn: assign handler_id=12345 + Dmn->>-Bus: reply (t) 12345 + Bus->>-Lib: GVariant reply + Lib-->>Lib: ⬜ malloc handle, internal_system_init() + Lib-->>BG: ⬜ pthread_create + BG-->>Bus: subscribe to 3 signals + BG-->>BG: ⬜ g_main_loop_run() + Lib->>-App: 🟢 handle "12345" + + Note over App,HAL: Phase 2 — Check for Update + + App->>+Lib: checkForUpdate(handle, my_cb) + Lib-->>Lib: 🟡 validate handle + callback + Lib-->>Lib: ⬜ register cb in g_registry [PENDING] + Lib->>Bus: CheckForUpdate(s "12345") [fire-and-forget] + Lib->>-App: 🟢 CHECK_FOR_UPDATE_SUCCESS + App-->>App: pthread_cond_timedwait (120s) + + Bus->>Dmn: CheckForUpdate + Dmn->>+XConf: HTTP GET /xconf?model=... + XConf->>-Dmn: firmware_v2.bin available + + Dmn->>Bus: signal: CheckForUpdateComplete(t,i,i,s,s,s,s) + Bus->>BG: deliver signal + BG-->>BG: ⬜ on_check_complete_signal() + BG-->>BG: ⬜ dispatch_all_pending() + BG->>App: 🟢 my_cb(&fwinfo) [status=AVAILABLE] + App-->>App: pthread_cond_signal (wake main) + + Note over App,HAL: Phase 3 — Download + + App->>+Lib: downloadFirmware(handle, req, dl_cb) + Lib-->>Lib: 🟡 validate inputs + Lib-->>Lib: ⬜ register dl_cb in g_dwnl_registry [ACTIVE] + Lib->>Bus: DownloadFirmware(ssss) [fire-and-forget] + Lib->>-App: 🟢 RDKFW_DWNL_SUCCESS + App-->>App: pthread_cond_timedwait (300s) + + Bus->>Dmn: DownloadFirmware + Dmn->>+CDN: HTTPS GET firmware_v2.bin + + loop Every progress update + CDN-->>Dmn: chunk received + Dmn->>Bus: signal: DownloadProgress(t,s,u,s,s) + Bus->>BG: deliver signal + BG->>App: dl_cb(progress%, IN_PROGRESS) + end + + CDN->>-Dmn: download complete + Dmn->>Bus: signal: DownloadProgress(100, COMPLETED) + Bus->>BG: deliver signal + BG->>App: 🟢 dl_cb(100, COMPLETED) + App-->>App: pthread_cond_signal + + Note over App,HAL: Phase 4 — Flash Update + + App->>+Lib: updateFirmware(handle, req, upd_cb) + Lib-->>Lib: 🟡 validate inputs + Lib-->>Lib: ⬜ register upd_cb in g_update_registry [ACTIVE] + Lib->>Bus: UpdateFirmware(sssss) [fire-and-forget] + Lib->>-App: 🟢 RDKFW_UPDATE_SUCCESS + App-->>App: pthread_cond_timedwait (600s) + + Bus->>Dmn: UpdateFirmware + Dmn->>+HAL: flash firmware_v2.bin + + loop Flash progress + HAL-->>Dmn: partition written + Dmn->>Bus: signal: UpdateProgress(t,s,i,i,s) + Bus->>BG: deliver signal + BG->>App: upd_cb(progress%, IN_PROGRESS) + end + + HAL->>-Dmn: flash complete + Dmn->>Bus: signal: UpdateProgress(100, COMPLETED) + Bus->>BG: deliver signal + BG->>App: 🟢 upd_cb(100, COMPLETED) + + Note over App,HAL: Phase 5 — Cleanup + + App->>+Lib: unregisterProcess(handle) + Lib-->>Lib: ⬜ internal_system_deinit() + Lib-->>BG: g_main_loop_quit() + BG-->>Lib: thread exits + Lib-->>Lib: ⬜ pthread_join, free registries + Lib->>Bus: UnregisterProcess(t 12345) + Bus->>Dmn: UnregisterProcess + Dmn-->>Dmn: remove ProcessInfo + Lib-->>Lib: ⬜ free(handle) + Lib->>-App: 🟢 return (void) +``` + +--- + +### 3.2 Daemon Unavailable + Retry + +```mermaid +sequenceDiagram + autonumber + participant App as 🔵 Client App + participant Lib as 🔵 Library + participant Bus as ⬜ D-Bus + + Note over App,Bus: Attempt 1 — Daemon not running + + App->>+Lib: registerProcess("MyPlugin", "1.0") + Lib-->>Lib: 🟡 validate inputs — OK + Lib->>+Bus: g_bus_get_sync(SYSTEM) + Bus->>-Lib: connection OK + Lib->>+Bus: g_dbus_proxy_new_sync(org.rdkfwupdater.Service) + Bus->>-Lib: 🔴 Error: "name not provided by any .service files" + Lib-->>Lib: 🟣 FWUPMGR_ERROR: proxy creation failed + Lib-->>Lib: ⬜ g_object_unref(connection) + Lib->>-App: 🔴 return NULL + + App-->>App: Log "daemon not running" + App-->>App: sleep(5) + + Note over App,Bus: Attempt 2 — Daemon started between retries + + App->>+Lib: registerProcess("MyPlugin", "1.0") + Lib-->>Lib: 🟡 validate inputs — OK + Lib->>+Bus: g_bus_get_sync(SYSTEM) + Bus->>-Lib: connection OK + Lib->>+Bus: g_dbus_proxy_new_sync(org.rdkfwupdater.Service) + Bus->>-Lib: 🟢 proxy OK + Lib->>+Bus: RegisterProcess(ss) + Bus->>-Lib: 🟢 handler_id = 42 + Lib-->>Lib: ⬜ internal_system_init() + Lib->>-App: 🟢 handle "42" + App-->>App: proceed with firmware operations +``` + +--- + +### 3.3 Callback Registration & Delivery + +```mermaid +sequenceDiagram + autonumber + participant Caller as 🔵 Caller Thread + participant API as 🔵 API Layer + participant Reg as ⬜ Registry (mutex) + participant Bus as ⬜ D-Bus + participant BG as ⬜ BG Thread + participant Dmn as ⬜ Daemon + + Note over Caller,Dmn: Step 1 — Register callback BEFORE sending D-Bus call + + Caller->>+API: checkForUpdate(handle, my_cb) + API->>+Reg: 🔒 lock(g_registry.mutex) + Reg-->>Reg: find IDLE slot + Reg-->>Reg: store {callback=my_cb, handle_key=handle, state=PENDING} + API->>-Reg: 🔓 unlock + API->>Bus: fire-and-forget: CheckForUpdate(handle) + API->>-Caller: 🟢 SUCCESS + + Note over Caller,Dmn: Step 2 — Signal arrives, two-phase dispatch + + Dmn->>Bus: signal: CheckForUpdateComplete(handler_id, ...) + Bus->>+BG: on_check_complete_signal() + + BG->>+Reg: 🔒 lock(g_registry.mutex) + Note over BG,Reg: Phase 1: Snapshot matching entries
Copy callback pointers + data to local array
Mark slots DISPATCHED + BG->>-Reg: 🔓 unlock + + Note over BG: Phase 2: Dispatch WITHOUT holding lock + BG->>Caller: my_cb(&fwinfo) — runs in BG thread context + Note over Caller: Callback copies data, signals condvar + + BG->>+Reg: 🔒 lock(g_registry.mutex) + Note over BG,Reg: Phase 3: Reset dispatched slots to IDLE + BG->>-Reg: 🔓 unlock + deactivate BG +``` + +--- + +### 3.4 Timeout Recovery + +```mermaid +sequenceDiagram + autonumber + participant App as 🔵 Client App + participant Lib as 🔵 Library + participant BG as ⬜ BG Thread + participant Dmn as ⬜ Daemon + + App->>Lib: checkForUpdate(handle, cb) + Lib-->>Lib: register cb [PENDING] + Lib->>Dmn: CheckForUpdate (fire-and-forget) + Lib->>App: 🟢 SUCCESS + + App-->>App: pthread_cond_timedwait(120s) + + Note over Dmn: Daemon is overloaded or
XConf is unreachable + + Note over App: ⏰ 120 seconds pass...
No signal arrives + + App-->>App: 🔴 timedwait returns ETIMEDOUT + + Note over App: Recovery: clean up and retry + + App->>+Lib: unregisterProcess(handle) + Lib-->>Lib: internal_system_deinit() + Lib-->>BG: g_main_loop_quit() + BG-->>Lib: thread exits + Lib-->>Lib: free registries (stale PENDING cleared) + Lib->>Dmn: UnregisterProcess + Lib-->>Lib: free(handle) + Lib->>-App: return + + App-->>App: sleep(30) — backoff + + App->>+Lib: registerProcess("MyPlugin", "1.0") + Lib->>Dmn: RegisterProcess + Dmn->>Lib: handler_id=99 + Lib-->>Lib: internal_system_init() + Lib->>-App: 🟢 handle "99" + + App->>Lib: checkForUpdate("99", cb) + Lib->>Dmn: CheckForUpdate + Lib->>App: 🟢 SUCCESS + App-->>App: pthread_cond_timedwait(120s) + + Dmn->>BG: signal: CheckForUpdateComplete + BG->>App: 🟢 cb(&fwinfo) + App-->>App: condvar signaled — success +``` + +--- + +## 4. Thread Safety Diagram + +### 4.1 Multi-Client Shared State Map + +```mermaid +graph TB + subgraph PROCESS["Single Process Address Space"] + subgraph MAIN["🔵 Main Thread"] + MT_REG["registerProcess()"] + MT_CHECK["checkForUpdate()"] + MT_DWNL["downloadFirmware()"] + MT_UPD["updateFirmware()"] + MT_UNREG["unregisterProcess()"] + end + + subgraph WORKER["🔵 Worker Thread (optional)"] + WT_CHECK["checkForUpdate()"] + WT_DWNL["downloadFirmware()"] + end + + subgraph SHARED["⬜ Shared State (mutex-protected)"] + subgraph REG1["g_registry
🔒 registry_mutex"] + R1_S["slots[0..29]
state | callback | handle_key"] + end + subgraph REG2["g_dwnl_registry
🔒 dwnl_mutex"] + R2_S["slots[0..29]
state | callback | handle_key"] + end + subgraph REG3["g_update_registry
🔒 update_mutex"] + R3_S["slots[0..29]
state | callback | handle_key"] + end + end + + subgraph BG_THREAD["⬜ Background Thread"] + LOOP["g_main_loop_run()"] + SIG1["on_check_complete_signal()"] + SIG2["on_download_progress_signal()"] + SIG3["on_update_progress_signal()"] + LOOP --> SIG1 + LOOP --> SIG2 + LOOP --> SIG3 + end + + MT_CHECK -- "🔒 lock → store → unlock" --> REG1 + WT_CHECK -- "🔒 lock → store → unlock" --> REG1 + SIG1 -- "🔒 lock → snapshot → unlock
dispatch WITHOUT lock
🔒 lock → reset → unlock" --> REG1 + + MT_DWNL -- "🔒 lock → store → unlock" --> REG2 + WT_DWNL -- "🔒 lock → store → unlock" --> REG2 + SIG2 -- "🔒 lock → snapshot → unlock
dispatch WITHOUT lock
🔒 lock → reset → unlock" --> REG2 + + MT_UPD -- "🔒 lock → store → unlock" --> REG3 + SIG3 -- "🔒 lock → snapshot → unlock
dispatch WITHOUT lock
🔒 lock → reset → unlock" --> REG3 + end + + style MAIN fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style WORKER fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style SHARED fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style BG_THREAD fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style REG1 fill:#fef3c7,stroke:#d97706,stroke-width:2px,color:#78350f + style REG2 fill:#fef3c7,stroke:#d97706,stroke-width:2px,color:#78350f + style REG3 fill:#fef3c7,stroke:#d97706,stroke-width:2px,color:#78350f +``` + +### 4.2 Two-Phase Dispatch (Deadlock Prevention) + +```mermaid +flowchart LR + subgraph PHASE1["Phase 1: Under Lock"] + direction TB + P1_LOCK["🔒 mutex_lock"] + P1_SCAN["Scan slots:
find matching handler_id"] + P1_COPY["Copy callback ptr + data
to stack-local snapshot[]"] + P1_MARK["Mark slot DISPATCHED"] + P1_UNLOCK["🔓 mutex_unlock"] + P1_LOCK --> P1_SCAN --> P1_COPY --> P1_MARK --> P1_UNLOCK + end + + subgraph PHASE2["Phase 2: No Lock Held"] + direction TB + P2_LOOP["for each snapshot entry"] + P2_CALL["snapshot[i].callback(data)"] + P2_NOTE["Client callback executes
freely — can call any API
NO DEADLOCK RISK"] + P2_LOOP --> P2_CALL --> P2_NOTE + end + + subgraph PHASE3["Phase 3: Under Lock"] + direction TB + P3_LOCK["🔒 mutex_lock"] + P3_RESET["Reset DISPATCHED → IDLE
free handle_key"] + P3_UNLOCK["🔓 mutex_unlock"] + P3_LOCK --> P3_RESET --> P3_UNLOCK + end + + PHASE1 --> PHASE2 --> PHASE3 + + style PHASE1 fill:#fef3c7,stroke:#d97706,stroke-width:2px,color:#78350f + style PHASE2 fill:#dcfce7,stroke:#16a34a,stroke-width:2px,color:#14532d + style PHASE3 fill:#fef3c7,stroke:#d97706,stroke-width:2px,color:#78350f +``` + +### 4.3 Connection Model — Why Each Call Is Independent + +```mermaid +sequenceDiagram + participant App as 🔵 Client + participant Lib as 🔵 Library + participant Bus as ⬜ D-Bus + + Note over App,Bus: Each API call creates + destroys its own connection + + App->>Lib: registerProcess() + Lib->>+Bus: g_bus_get_sync() → conn_1 (sender :1.140) + Lib->>Bus: RegisterProcess via conn_1 + Lib->>-Bus: g_object_unref(conn_1) — destroyed + + App->>Lib: checkForUpdate() + Lib->>+Bus: g_bus_get_sync() → conn_2 (sender :1.141) + Lib->>Bus: CheckForUpdate via conn_2 + Lib->>-Bus: g_object_unref(conn_2) — destroyed + + App->>Lib: downloadFirmware() + Lib->>+Bus: g_bus_get_sync() → conn_3 (sender :1.142) + Lib->>Bus: DownloadFirmware via conn_3 + Lib->>-Bus: g_object_unref(conn_3) — destroyed + + Note over Lib,Bus: BG Thread has separate PERSISTENT connection
for signal subscription (lives entire session) +``` + +--- + +## 5. Memory Ownership Diagram + +### 5.1 Ownership Boundaries + +```mermaid +graph TB + subgraph CALLER["🔵 Caller Allocates & Owns"] + C1["processName (const char*)
passed to registerProcess()"] + C2["libVersion (const char*)
passed to registerProcess()"] + C3["FwDwnlReq struct
passed to downloadFirmware()"] + C4["FwUpdateReq struct
passed to updateFirmware()"] + C5["callback function pointers"] + C6["condvar / mutex
for synchronization"] + C7["Copies of callback data
(strncpy'd inside callback)"] + end + + subgraph LIBRARY["🔵 Library Allocates & Owns"] + L1["FirmwareInterfaceHandle
malloc(32) in registerProcess()"] + L2["handle_key strings
strdup'd in registry slots"] + L3["CallbackRegistry
(static global array)"] + L4["DwnlCallbackRegistry
(static global array)"] + L5["UpdateCbRegistry
(static global array)"] + L6["GMainContext
+ GMainLoop"] + L7["Background pthread"] + L8["FwInfoData
(stack in dispatch — transient!)"] + L9["UpdateDetails
(stack in dispatch — transient!)"] + end + + subgraph DAEMON["⬜ Daemon Allocates & Owns"] + D1["handler_id counter
(uint64)"] + D2["ProcessInfo records
(GHashTable)"] + D3["GVariant signal payloads
(auto-freed after emission)"] + D4["Downloaded firmware file
(on-disk)"] + end + + subgraph DBUS["⬜ D-Bus / GLib Manages"] + B1["GDBusConnection
(per-call ephemeral)"] + B2["GDBusProxy objects"] + B3["GVariant method args
+ reply data"] + B4["BG thread persistent
GDBusConnection"] + end + + L1 -. "freed by
unregisterProcess()" .-> FREE1(("free()")) + L2 -. "freed on slot reset" .-> FREE2(("free()")) + L6 -. "unref'd in
internal_system_deinit()" .-> FREE3(("unref")) + B1 -. "unref'd after each API call" .-> FREE4(("unref")) + B2 -. "unref'd after each API call" .-> FREE5(("unref")) + + style CALLER fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style LIBRARY fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style DAEMON fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style DBUS fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style FREE1 fill:#ef4444,stroke:#b91c1c,color:#fff + style FREE2 fill:#ef4444,stroke:#b91c1c,color:#fff + style FREE3 fill:#ef4444,stroke:#b91c1c,color:#fff + style FREE4 fill:#ef4444,stroke:#b91c1c,color:#fff + style FREE5 fill:#ef4444,stroke:#b91c1c,color:#fff +``` + +### 5.2 Callback Data Lifetime + +```mermaid +flowchart LR + subgraph SIGNAL["Signal Arrives"] + SIG["D-Bus signal
GVariant payload"] + end + + subgraph PARSE["BG Thread Parses"] + P1["g_variant_get()
extract fields"] + P2["Populate FwInfoData
on STACK"] + P3["Populate UpdateDetails
on STACK"] + end + + subgraph DISPATCH["Callback Executes"] + CB["client_callback(&fwinfo)"] + COPY["🟢 Client MUST copy
any needed data NOW"] + WARN["🔴 After callback returns:
all pointers are INVALID"] + end + + subgraph CLEANUP["Stack Unwinds"] + CL["FwInfoData destroyed
UpdateDetails destroyed
GVariant unreffed"] + end + + SIG --> PARSE --> DISPATCH --> CLEANUP + + style SIGNAL fill:#f3f4f6,stroke:#6b7280,color:#374151 + style PARSE fill:#f3f4f6,stroke:#6b7280,color:#374151 + style DISPATCH fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style CLEANUP fill:#fecaca,stroke:#dc2626,color:#7f1d1d + style COPY fill:#22c55e,stroke:#15803d,color:#fff + style WARN fill:#ef4444,stroke:#b91c1c,color:#fff +``` + +### 5.3 Cleanup Sequence + +```mermaid +flowchart TD + UNREG(["🔵 unregisterProcess(handle)"]) + UNREG --> DEINIT["⬜ internal_system_deinit()"] + + DEINIT --> S1["⬜ g_main_loop_quit()
→ BG thread wakes up"] + S1 --> S2["⬜ pthread_join()
→ BG thread fully stopped"] + S2 --> S3["⬜ g_main_loop_unref()
g_main_context_unref()"] + S3 --> S4["⬜ internal_dwnl_system_deinit()
→ free dwnl registry handle_keys
→ destroy dwnl_mutex"] + S4 --> S5["⬜ internal_update_system_deinit()
→ free update registry handle_keys
→ destroy update_mutex"] + S5 --> S6["⬜ Free check registry handle_keys"] + S6 --> S7["⬜ pthread_mutex_destroy × 4
(bg_thread, registry, dwnl, update)"] + S7 --> S8["⬜ D-Bus: UnregisterProcess
(best-effort)"] + S8 --> S9["⬜ free(handle)
🔴 handle pointer now INVALID"] + S9 --> DONE(["🟢 All resources released"]) + + style UNREG fill:#3b82f6,stroke:#1d4ed8,color:#fff + style DONE fill:#22c55e,stroke:#15803d,color:#fff + style S9 fill:#ef4444,stroke:#b91c1c,color:#fff + style DEINIT fill:#9ca3af,stroke:#4b5563,color:#fff + style S1 fill:#9ca3af,stroke:#4b5563,color:#fff + style S2 fill:#9ca3af,stroke:#4b5563,color:#fff + style S3 fill:#9ca3af,stroke:#4b5563,color:#fff + style S4 fill:#9ca3af,stroke:#4b5563,color:#fff + style S5 fill:#9ca3af,stroke:#4b5563,color:#fff + style S6 fill:#9ca3af,stroke:#4b5563,color:#fff + style S7 fill:#9ca3af,stroke:#4b5563,color:#fff + style S8 fill:#9ca3af,stroke:#4b5563,color:#fff +``` + +--- + +## 6. Logging Pipeline Diagram + +### 6.1 Three-Module Logging Architecture + +```mermaid +graph TB + subgraph SOURCES["Log Sources"] + direction TB + subgraph CLIENT_LOG["🔵 Client Application"] + EX_INFO["EXAMPLE_INFO(...)"] + EX_ERR["EXAMPLE_ERROR(...)"] + EX_DBG["EXAMPLE_DEBUG(...)"] + end + + subgraph LIB_LOG["🔵 Library Internals"] + FW_INFO["FWUPMGR_INFO(...)"] + FW_ERR["FWUPMGR_ERROR(...)"] + FW_WARN["FWUPMGR_WARN(...)"] + FW_DBG["FWUPMGR_DEBUG(...)"] + end + + subgraph DMN_LOG["⬜ Daemon"] + SW_INFO["SWLOG_INFO(...)"] + SW_ERR["SWLOG_ERROR(...)"] + end + end + + subgraph MACROS["🟣 Macro Expansion Layer"] + M_CLIENT["Module: LOG.RDK.EXAMPLE"] + M_LIB["Module: LOG.RDK.FWUPMGR"] + M_DMN["Module: LOG.RDK.FWUPG"] + end + + subgraph BACKEND["🟣 Logging Backend"] + RDK{"RDK_LOGGER
defined?"} + YES["RDK_LOG(level, module, fmt, ...)
→ rdk_logger subsystem"] + NO["fprintf(stderr, [module] fmt, ...)
→ console fallback"] + end + + subgraph OUTPUT["🟣 Log Output"] + FILE["/opt/logs/rdkFwupdateMgr.log"] + CONSOLE["stderr (unit tests)"] + end + + EX_INFO & EX_ERR & EX_DBG --> M_CLIENT + FW_INFO & FW_ERR & FW_WARN & FW_DBG --> M_LIB + SW_INFO & SW_ERR --> M_DMN + + M_CLIENT & M_LIB & M_DMN --> RDK + + RDK -- "Yes (production)" --> YES --> FILE + RDK -- "No (unit test)" --> NO --> CONSOLE + + style CLIENT_LOG fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style LIB_LOG fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style DMN_LOG fill:#f3f4f6,stroke:#6b7280,stroke-width:2px,color:#374151 + style MACROS fill:#f3e8ff,stroke:#9333ea,stroke-width:2px,color:#581c87 + style BACKEND fill:#f3e8ff,stroke:#9333ea,stroke-width:2px,color:#581c87 + style OUTPUT fill:#f3e8ff,stroke:#9333ea,stroke-width:2px,color:#581c87 + style RDK fill:#a855f7,stroke:#7e22ce,color:#fff +``` + +### 6.2 Log Points by API Function + +```mermaid +graph LR + subgraph REGISTER["registerProcess()"] + R_ENTRY["🟣 INFO: Entry
(processName)"] + R_VAL["🟣 ERROR: Validation
failure details"] + R_DBUS["🟣 ERROR: D-Bus
connection/call failure"] + R_ID["🟣 INFO: Got handler_id"] + R_INIT["🟣 INFO: BG thread started"] + R_OK["🟣 INFO: Registered
handle=X"] + end + + subgraph CHECK["checkForUpdate()"] + C_ENTRY["🟣 INFO: Entry
(handle)"] + C_VAL["🟣 ERROR: Bad handle
or NULL callback"] + C_REG["🟣 DEBUG: Slot allocated
in registry"] + C_SEND["🟣 INFO: Request sent"] + end + + subgraph DOWNLOAD["downloadFirmware()"] + D_ENTRY["🟣 INFO: Entry
(handle, firmwareName)"] + D_VAL["🟣 ERROR: Missing
required fields"] + D_REG["🟣 DEBUG: Slot allocated
in dwnl_registry"] + D_SEND["🟣 INFO: Request sent"] + end + + subgraph UPDATE["updateFirmware()"] + U_ENTRY["🟣 INFO: Entry
(handle, firmwareName)"] + U_VAL["🟣 ERROR: Missing
name or type"] + U_REG["🟣 DEBUG: Slot allocated"] + U_SEND["🟣 INFO: Request sent"] + end + + subgraph UNREGISTER["unregisterProcess()"] + X_ENTRY["🟣 INFO: Entry
(handle)"] + X_PARSE["🟣 ERROR: Invalid
handle format"] + X_DEINIT["🟣 INFO: Deinit
starting"] + X_WARN["🟣 WARN: D-Bus call
failed (best-effort)"] + X_OK["🟣 INFO: Unregistered OK"] + end + + subgraph BG_SIGNALS["Background Thread Signals"] + S_RECV["🟣 DEBUG: Signal received
(handler_id, type)"] + S_DISPATCH["🟣 DEBUG: Dispatching
N callbacks"] + S_RESET["🟣 DEBUG: Slot reset
to IDLE"] + end + + style REGISTER fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style CHECK fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style DOWNLOAD fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style UPDATE fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style UNREGISTER fill:#dbeafe,stroke:#2563eb,color:#1e3a5f + style BG_SIGNALS fill:#f3f4f6,stroke:#6b7280,color:#374151 + + style R_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style R_VAL fill:#a855f7,stroke:#7e22ce,color:#fff + style R_DBUS fill:#a855f7,stroke:#7e22ce,color:#fff + style R_ID fill:#a855f7,stroke:#7e22ce,color:#fff + style R_INIT fill:#a855f7,stroke:#7e22ce,color:#fff + style R_OK fill:#a855f7,stroke:#7e22ce,color:#fff + style C_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style C_VAL fill:#a855f7,stroke:#7e22ce,color:#fff + style C_REG fill:#a855f7,stroke:#7e22ce,color:#fff + style C_SEND fill:#a855f7,stroke:#7e22ce,color:#fff + style D_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style D_VAL fill:#a855f7,stroke:#7e22ce,color:#fff + style D_REG fill:#a855f7,stroke:#7e22ce,color:#fff + style D_SEND fill:#a855f7,stroke:#7e22ce,color:#fff + style U_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style U_VAL fill:#a855f7,stroke:#7e22ce,color:#fff + style U_REG fill:#a855f7,stroke:#7e22ce,color:#fff + style U_SEND fill:#a855f7,stroke:#7e22ce,color:#fff + style X_ENTRY fill:#a855f7,stroke:#7e22ce,color:#fff + style X_PARSE fill:#a855f7,stroke:#7e22ce,color:#fff + style X_DEINIT fill:#a855f7,stroke:#7e22ce,color:#fff + style X_WARN fill:#a855f7,stroke:#7e22ce,color:#fff + style X_OK fill:#a855f7,stroke:#7e22ce,color:#fff + style S_RECV fill:#a855f7,stroke:#7e22ce,color:#fff + style S_DISPATCH fill:#a855f7,stroke:#7e22ce,color:#fff + style S_RESET fill:#a855f7,stroke:#7e22ce,color:#fff +``` + +### 6.3 Correlation — Tracing a Request by handler_id + +```mermaid +sequenceDiagram + participant App as 🔵 Client + participant Lib as 🔵 Library + participant Log as 🟣 Log File + participant Dmn as ⬜ Daemon + + App->>Lib: registerProcess("MyPlugin", "1.0") + Lib->>Log: [FWUPMGR] INFO: registerProcess entry, processName=MyPlugin + Lib->>Dmn: RegisterProcess + Dmn->>Log: [FWUPG] INFO: Registered MyPlugin, handler_id=12345 + Lib->>Log: [FWUPMGR] INFO: Got handler_id=12345 + Lib->>App: handle "12345" + App->>Log: [EXAMPLE] INFO: Registered with handle 12345 + + Note over Log: All subsequent logs include handler_id=12345
for end-to-end request correlation + + App->>Lib: checkForUpdate("12345", cb) + Lib->>Log: [FWUPMGR] INFO: checkForUpdate, handle=12345 + Lib->>Log: [FWUPMGR] DEBUG: registry slot 3 allocated, handler_id=12345 + Lib->>Dmn: CheckForUpdate("12345") + Dmn->>Log: [FWUPG] INFO: CheckForUpdate for handler_id=12345, querying XConf + + Note over Dmn: XConf responds + + Dmn->>Log: [FWUPG] INFO: CheckForUpdate result for 12345: AVAILABLE + Dmn->>Lib: signal CheckForUpdateComplete(12345, ...) + Lib->>Log: [FWUPMGR] DEBUG: signal received, handler_id=12345 + Lib->>Log: [FWUPMGR] DEBUG: dispatching 1 callback(s) for handler_id=12345 + Lib->>App: cb(&fwinfo) + App->>Log: [EXAMPLE] INFO: Firmware available for handle 12345 +``` + +### 6.4 Log Lifecycle Ownership + +```mermaid +flowchart TD + subgraph APP["🔵 Client Application (caller's responsibility)"] + INIT["log_init()
⚠️ MUST call before any library API"] + USE["Use library APIs
(all logging works)"] + EXIT["log_exit()
⚠️ MUST call after unregisterProcess()"] + INIT --> USE --> EXIT + end + + subgraph LIB_INTERNAL["🔵 Library (never calls log_init/exit)"] + LOG_CALL["FWUPMGR_INFO/ERROR/DEBUG/WARN
Just emits — assumes log is initialized"] + end + + USE -.-> LOG_CALL + + subgraph WRONG["🔴 WRONG — Double Init"] + BAD["Library calling log_init()
→ corrupts app's log state"] + end + + style APP fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style LIB_INTERNAL fill:#dbeafe,stroke:#2563eb,stroke-width:2px,color:#1e3a5f + style WRONG fill:#fecaca,stroke:#dc2626,stroke-width:2px,color:#7f1d1d + style INIT fill:#22c55e,stroke:#15803d,color:#fff + style EXIT fill:#22c55e,stroke:#15803d,color:#fff + style BAD fill:#ef4444,stroke:#b91c1c,color:#fff +``` + +--- + +## Appendix: State Machine Diagrams + +### A.1 Check Callback Registry Slot States + +```mermaid +stateDiagram-v2 + [*] --> IDLE + + IDLE --> PENDING : checkForUpdate()
registers callback + PENDING --> DISPATCHED : Signal arrives
Phase 1 snapshots + DISPATCHED --> IDLE : Phase 3 resets
after callback returns + PENDING --> TIMED_OUT : Future: sweep thread
(not yet implemented) + TIMED_OUT --> IDLE : Cleanup + + state IDLE { + [*] : Slot available + } + state PENDING { + [*] : Callback stored, waiting for signal + } + state DISPATCHED { + [*] : Callback is being invoked + } + state TIMED_OUT { + [*] : Stale entry (future) + } +``` + +### A.2 Download/Update Registry Slot States + +```mermaid +stateDiagram-v2 + [*] --> IDLE + + IDLE --> ACTIVE : downloadFirmware() /
updateFirmware() + ACTIVE --> ACTIVE : Progress signal
(still in progress) + ACTIVE --> IDLE : Terminal signal
(COMPLETED or ERROR) + + state IDLE { + [*] : Slot available + } + state ACTIVE { + [*] : Callback registered,
receiving progress signals + } +``` + +### A.3 Library Handle Lifecycle + +```mermaid +stateDiagram-v2 + [*] --> UNLINKED : Library loaded + + UNLINKED --> REGISTERED : registerProcess()
returns non-NULL handle + UNLINKED --> UNLINKED : registerProcess()
returns NULL (error) + + REGISTERED --> ACTIVE : checkForUpdate() /
downloadFirmware() /
updateFirmware() + ACTIVE --> ACTIVE : More API calls + ACTIVE --> REGISTERED : All callbacks complete + + REGISTERED --> UNLINKED : unregisterProcess() + ACTIVE --> UNLINKED : unregisterProcess()
(stale callbacks cleared) + + state UNLINKED { + [*] : No handle, no BG thread + } + state REGISTERED { + [*] : Handle valid,
BG thread running,
no pending ops + } + state ACTIVE { + [*] : Handle valid,
pending callbacks
in registries + } +``` + +--- + +*End of Visual Engineering Documentation* diff --git a/librdkFwupdateMgr/examples/example_app.c b/librdkFwupdateMgr/examples/example_app.c index f2207880..2cdf0515 100644 --- a/librdkFwupdateMgr/examples/example_app.c +++ b/librdkFwupdateMgr/examples/example_app.c @@ -39,6 +39,18 @@ //#include "rdkFwupdateMgr_process.h" /* registerProcess(), unregisterProcess() */ #include "rdkFwupdateMgr_client.h" /* checkForUpdate(), downloadFirmware(), updateFirmware(), all callbacks/enums */ +#include "rdkFwupdateMgr_log.h" /* FWUPMGR_LOG() generic base macro */ +#include "rdkv_cdl_log_wrapper.h" /* log_init(), log_exit() */ + +/* ======================================================================== + * EXAMPLE_* logging macros use FWUPMGR_LOG with LOG.RDK.EXAMPLE module. + * Keeps example_plugin logs as [EXAMPLE], distinguishable from [FWUPMGR] + * library logs and [FWUPG] daemon logs. + * ======================================================================== */ +#define EXAMPLE_DEBUG(format, ...) FWUPMGR_LOG(RDK_LOG_DEBUG, "LOG.RDK.EXAMPLE", format, ##__VA_ARGS__) +#define EXAMPLE_INFO(format, ...) FWUPMGR_LOG(RDK_LOG_INFO, "LOG.RDK.EXAMPLE", format, ##__VA_ARGS__) +#define EXAMPLE_WARN(format, ...) FWUPMGR_LOG(RDK_LOG_WARN, "LOG.RDK.EXAMPLE", format, ##__VA_ARGS__) +#define EXAMPLE_ERROR(format, ...) FWUPMGR_LOG(RDK_LOG_ERROR, "LOG.RDK.EXAMPLE", format, ##__VA_ARGS__) #include #include #include @@ -104,11 +116,10 @@ static int g_exit_code = EXIT_SUCCESS; */ static void on_firmware_check_callback(const FwInfoData *event_data) { - printf("\n"); - printf("│ ✓ checkForUpdate Callback Received │\n"); + EXAMPLE_INFO("checkForUpdate Callback Received\n"); if (!event_data) { - fprintf(stderr, "[ERROR] event_data is NULL in callback!\n"); + EXAMPLE_ERROR("event_data is NULL in callback!\n"); pthread_mutex_lock(&g_check_mutex); g_check_status = FIRMWARE_CHECK_ERROR; g_check_done = 1; @@ -128,40 +139,40 @@ static void on_firmware_check_callback(const FwInfoData *event_data) case BYPASS_OPTOUT: status_str = "BYPASS_OPTOUT"; break; } - printf("\n === Basic Firmware Info ===\n"); - printf(" Handle : %s\n", g_handle ? g_handle : "(null)"); - printf(" Status Code : %s (%d)\n", status_str, event_data->status); - printf(" Current FW Version : %s\n", + EXAMPLE_INFO("=== Basic Firmware Info ===\n"); + EXAMPLE_INFO(" Handle : %s\n", g_handle ? g_handle : "(null)"); + EXAMPLE_INFO(" Status Code : %s (%d)\n", status_str, event_data->status); + EXAMPLE_INFO(" Current FW Version : %s\n", event_data->CurrFWVersion[0] ? event_data->CurrFWVersion : "(not provided)"); /* Print UpdateDetails if available (only when status == FIRMWARE_AVAILABLE) */ if (event_data->status == FIRMWARE_AVAILABLE && event_data->UpdateDetails) { - printf("\n === Update Details (Available!) ===\n"); - printf(" FwFileName : %s\n", + EXAMPLE_INFO("=== Update Details (Available!) ===\n"); + EXAMPLE_INFO(" FwFileName : %s\n", event_data->UpdateDetails->FwFileName[0] ? event_data->UpdateDetails->FwFileName : "null"); - printf(" FwUrl : %s\n", + EXAMPLE_INFO(" FwUrl : %s\n", event_data->UpdateDetails->FwUrl[0] ? event_data->UpdateDetails->FwUrl : "null"); - printf(" FwVersion : %s\n", + EXAMPLE_INFO(" FwVersion : %s\n", event_data->UpdateDetails->FwVersion[0] ? event_data->UpdateDetails->FwVersion : "null"); - printf(" RebootImmediately : %s\n", + EXAMPLE_INFO(" RebootImmediately : %s\n", event_data->UpdateDetails->RebootImmediately[0] ? event_data->UpdateDetails->RebootImmediately : "null"); - printf(" DelayDownload : %s\n", + EXAMPLE_INFO(" DelayDownload : %s\n", event_data->UpdateDetails->DelayDownload[0] ? event_data->UpdateDetails->DelayDownload : "null"); - printf(" PDRIVersion : %s\n", + EXAMPLE_INFO(" PDRIVersion : %s\n", event_data->UpdateDetails->PDRIVersion[0] ? event_data->UpdateDetails->PDRIVersion : "null"); - printf(" PeripheralFirmwares : %s\n", + EXAMPLE_INFO(" PeripheralFirmwares : %s\n", event_data->UpdateDetails->PeripheralFirmwares[0] ? event_data->UpdateDetails->PeripheralFirmwares : "null"); } else if (event_data->status == FIRMWARE_AVAILABLE && !event_data->UpdateDetails) { - printf("\n ⚠ WARNING: Status is FIRMWARE_AVAILABLE but UpdateDetails is NULL!\n"); + EXAMPLE_WARN("Status is FIRMWARE_AVAILABLE but UpdateDetails is NULL!\n"); } else { - printf("\n → No update details (status != FIRMWARE_AVAILABLE)\n"); + EXAMPLE_INFO("No update details (status != FIRMWARE_AVAILABLE)\n"); } /* Copy data to global state (data is only valid during this callback!) */ @@ -219,7 +230,7 @@ static void on_firmware_check_callback(const FwInfoData *event_data) pthread_cond_signal(&g_check_cond); pthread_mutex_unlock(&g_check_mutex); - printf("\n → Firmware check data saved. Main thread will proceed.\n"); + EXAMPLE_INFO("Firmware check data saved. Main thread will proceed.\n"); } /* ======================================================================== @@ -246,13 +257,8 @@ static void on_download_progress_callback(int progress_per, DownloadStatus fwdwn case DWNL_ERROR: status_str = "DWNL_ERROR"; break; } - /* Print progress bar: [████████░░░░░░░░░░░░] 40% DWNL_IN_PROGRESS */ - int bar_filled = progress_per / 5; /* 20 characters = 100% */ - printf(" ["); - for (int i = 0; i < 20; i++) { - printf(i < bar_filled ? "█" : "░"); - } - printf("] %3d%% %s\n", progress_per, status_str); + /* Print progress: 40% DWNL_IN_PROGRESS */ + EXAMPLE_INFO(" Download: %3d%% %s\n", progress_per, status_str); /* On terminal states (COMPLETED or ERROR), wake main thread */ if (fwdwnlstatus == DWNL_COMPLETED || fwdwnlstatus == DWNL_ERROR) { @@ -263,9 +269,9 @@ static void on_download_progress_callback(int progress_per, DownloadStatus fwdwn pthread_mutex_unlock(&g_download_mutex); if (fwdwnlstatus == DWNL_COMPLETED) { - printf("\n ✓ Download completed successfully!\n\n"); + EXAMPLE_INFO(" Download completed successfully!\n"); } else { - printf("\n ✗ Download failed!\n\n"); + EXAMPLE_ERROR(" Download failed!\n"); } } } @@ -294,13 +300,8 @@ static void on_update_progress_callback(int progress_per, UpdateStatus fwupdates case UPDATE_ERROR: status_str = "UPDATE_ERROR"; break; } - /* Print progress bar: [████████░░░░░░░░░░░░] 40% UPDATE_IN_PROGRESS */ - int bar_filled = progress_per / 5; /* 20 characters = 100% */ - printf(" ["); - for (int i = 0; i < 20; i++) { - printf(i < bar_filled ? "▓" : "░"); - } - printf("] %3d%% %s\n", progress_per, status_str); + /* Print progress: 40% UPDATE_IN_PROGRESS */ + EXAMPLE_INFO(" Flash: %3d%% %s\n", progress_per, status_str); /* On terminal states (COMPLETED or ERROR), wake main thread */ if (fwupdatestatus == UPDATE_COMPLETED || fwupdatestatus == UPDATE_ERROR) { @@ -311,9 +312,9 @@ static void on_update_progress_callback(int progress_per, UpdateStatus fwupdates pthread_mutex_unlock(&g_update_mutex); if (fwupdatestatus == UPDATE_COMPLETED) { - printf("\n ✓ Firmware flash completed successfully!\n\n"); + EXAMPLE_INFO(" Firmware flash completed successfully!\n"); } else { - printf("\n ✗ Firmware flash failed!\n\n"); + EXAMPLE_ERROR(" Firmware flash failed!\n"); } } } @@ -334,53 +335,57 @@ int main(void) struct timespec timeout; int rc; - printf("\n"); - printf("║ RDK Firmware Update Manager - Complete Workflow ║\n"); + /* Initialize logging must be first. + * All EXAMPLE_* and FWUPMGR_* log output goes to stdout/stderr. + * Shell redirect puts it in the right file: + * example_plugin > /opt/logs/rdkFwupdateMgr.log 2>&1 + */ + log_init(); + + EXAMPLE_INFO("==============================\n"); + EXAMPLE_INFO("Application starting, PID: %d\n", getpid()); /* ==================================================================== * STEP 1: Register Process with Daemon * ==================================================================== */ - printf("│ STEP 1: Register with firmware daemon │\n"); - printf(" Process Name : ExampleApp\n"); - printf(" Lib Version : 1.0.0\n\n"); + EXAMPLE_INFO("STEP 1: Register with firmware daemon\n"); + EXAMPLE_INFO(" Process Name : ExampleApp\n"); + EXAMPLE_INFO(" Lib Version : 1.0.0\n"); g_handle = registerProcess("ExampleApp", "1.0.0"); if (g_handle == NULL) { - fprintf(stderr, "[ERROR] registerProcess() failed!\n"); - fprintf(stderr, " Ensure rdkFwupdateMgr daemon is running:\n"); - fprintf(stderr, " systemctl status rdkFwupdateMgr.service\n\n"); + EXAMPLE_ERROR("registerProcess() failed!\n"); + EXAMPLE_ERROR("Ensure rdkFwupdateMgr daemon is running:\n"); + EXAMPLE_ERROR("systemctl status rdkFwupdateMgr.service\n"); + log_exit(); return EXIT_FAILURE; } - printf(" ✓ Registered successfully\n"); - printf(" Handle: '%s'\n\n", g_handle); + EXAMPLE_INFO("Registered successfully\n"); + EXAMPLE_INFO(" Handle: '%s'\n", g_handle); /* ==================================================================== * STEP 2: Check for Firmware Updates (Async) * ==================================================================== */ - printf("│ STEP 2: Check for firmware updates │\n"); - printf(" Calling checkForUpdate()...\n"); - printf(" (API returns immediately; callback fires when XConf query completes)\n\n"); + EXAMPLE_INFO("STEP 2: Check for firmware updates\n"); + EXAMPLE_INFO(" Calling checkForUpdate()...\n"); + EXAMPLE_INFO(" (API returns immediately; callback fires when XConf query completes)\n"); CheckForUpdateResult cfu_result = checkForUpdate(g_handle, on_firmware_check_callback); if (cfu_result != CHECK_FOR_UPDATE_SUCCESS) { - fprintf(stderr, "[ERROR] checkForUpdate() returned FAIL!\n"); - fprintf(stderr, " Possible reasons:\n"); - fprintf(stderr, " - D-Bus connection error\n"); - fprintf(stderr, " - Daemon not responding\n"); - fprintf(stderr, " - Invalid handle\n\n"); + EXAMPLE_ERROR("checkForUpdate() returned FAIL!\n"); + EXAMPLE_ERROR("Possible reasons: D-Bus error, daemon not responding, invalid handle\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } - printf(" ✓ checkForUpdate() returned SUCCESS\n"); - printf(" (Daemon ACK received - waiting for actual firmware data...)\n\n"); + EXAMPLE_INFO("checkForUpdate() returned SUCCESS\n"); + EXAMPLE_INFO(" (Daemon ACK received - waiting for actual firmware data...)\n"); /* Wait for callback with timeout (2 minutes for XConf query) */ - printf(" Waiting for firmware check callback"); - fflush(stdout); + EXAMPLE_INFO("Waiting for firmware check callback...\n"); clock_gettime(CLOCK_REALTIME, &timeout); timeout.tv_sec += 120; /* 2 minute timeout */ @@ -390,8 +395,8 @@ int main(void) rc = pthread_cond_timedwait(&g_check_cond, &g_check_mutex, &timeout); if (rc != 0) { pthread_mutex_unlock(&g_check_mutex); - fprintf(stderr, "\n[ERROR] Timeout waiting for checkForUpdate callback (120s)\n"); - fprintf(stderr, " XConf query may be taking longer than expected.\n\n"); + EXAMPLE_ERROR("Timeout waiting for checkForUpdate callback (120s)\n"); + EXAMPLE_ERROR("XConf query may be taking longer than expected.\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } @@ -399,31 +404,30 @@ int main(void) pthread_mutex_unlock(&g_check_mutex); /* Check result */ - printf("\n"); if (g_check_status != FIRMWARE_AVAILABLE) { - printf(" ⚠ No firmware update available\n"); - printf(" Status: %d\n", g_check_status); - printf(" Current Version: %s\n", g_fw_current_version); + EXAMPLE_WARN("No firmware update available\n"); + EXAMPLE_INFO(" Status: %d\n", g_check_status); + EXAMPLE_INFO(" Current Version: %s\n", g_fw_current_version); if (g_check_status == FIRMWARE_NOT_AVAILABLE) { - printf(" → Already on latest version. No action needed.\n\n"); + EXAMPLE_INFO(" Already on latest version. No action needed.\n"); g_exit_code = EXIT_SUCCESS; } else { - printf(" → Cannot proceed with update.\n\n"); + EXAMPLE_ERROR(" Cannot proceed with update.\n"); g_exit_code = EXIT_FAILURE; } goto cleanup_unregister; } - printf(" ✓ Firmware update available!\n"); - printf(" Current Version : %s\n", g_fw_current_version); - printf(" Available Version: %s\n", g_fw_available_version); - printf(" → Proceeding to download...\n\n"); + EXAMPLE_INFO("Firmware update available!\n"); + EXAMPLE_INFO(" Current Version : %s\n", g_fw_current_version); + EXAMPLE_INFO(" Available Version: %s\n", g_fw_available_version); + EXAMPLE_INFO(" Proceeding to download...\n"); /* ==================================================================== * STEP 3: Download Firmware (Async) * ==================================================================== */ - printf("│ STEP 3: Download firmware image │\n"); + EXAMPLE_INFO("STEP 3: Download firmware image\n"); /* Prepare download request using data from checkForUpdate callback */ FwDwnlReq download_req; @@ -437,24 +441,23 @@ int main(void) download_req.downloadUrl = fw_url; download_req.TypeOfFirmware = "PCI"; /* Default to PCI type */ - printf(" Firmware Name : %s\n", download_req.firmwareName); - printf(" Download URL : %s\n", download_req.downloadUrl[0] ? download_req.downloadUrl : "(use XConf URL)"); - printf(" Firmware Type : %s\n\n", download_req.TypeOfFirmware); + EXAMPLE_INFO(" Firmware Name : %s\n", download_req.firmwareName); + EXAMPLE_INFO(" Download URL : %s\n", download_req.downloadUrl[0] ? download_req.downloadUrl : "(use XConf URL)"); + EXAMPLE_INFO(" Firmware Type : %s\n", download_req.TypeOfFirmware); - printf(" Calling downloadFirmware()...\n\n"); + EXAMPLE_INFO(" Calling downloadFirmware()...\n"); DownloadResult dl_result = downloadFirmware(g_handle, &download_req, on_download_progress_callback); if (dl_result != RDKFW_DWNL_SUCCESS) { - fprintf(stderr, "[ERROR] downloadFirmware() returned FAIL!\n\n"); + EXAMPLE_ERROR("downloadFirmware() returned FAIL!\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } - printf(" ✓ downloadFirmware() returned SUCCESS\n"); - printf(" Waiting for download progress...\n\n"); - printf(" Download Progress:\n"); + EXAMPLE_INFO("downloadFirmware() returned SUCCESS\n"); + EXAMPLE_INFO(" Waiting for download progress...\n"); /* Wait for download completion with timeout (5 minutes) */ clock_gettime(CLOCK_REALTIME, &timeout); @@ -465,7 +468,7 @@ int main(void) rc = pthread_cond_timedwait(&g_download_cond, &g_download_mutex, &timeout); if (rc != 0) { pthread_mutex_unlock(&g_download_mutex); - fprintf(stderr, "[ERROR] Timeout waiting for download completion (5 min)\n\n"); + EXAMPLE_ERROR("Timeout waiting for download completion (5 min)\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } @@ -474,17 +477,17 @@ int main(void) /* Check download result */ if (g_download_status != DWNL_COMPLETED) { - fprintf(stderr, "[ERROR] Download failed (status=%d)\n\n", g_download_status); + EXAMPLE_ERROR("Download failed (status=%d)\n", g_download_status); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } - printf(" → Download complete. Proceeding to flash...\n\n"); + EXAMPLE_INFO("Download complete. Proceeding to flash...\n"); /* ==================================================================== * STEP 4: Update/Flash Firmware (Async) * ==================================================================== */ - printf("│ STEP 4: Flash firmware to device │\n"); + EXAMPLE_INFO("STEP 4: Flash firmware to device\n"); /* Prepare update request */ FwUpdateReq update_req; @@ -506,25 +509,24 @@ int main(void) /* Reboot after flash: false for this example (so we can unregister cleanly) */ update_req.rebootImmediately = false; - printf(" Firmware Name : %s\n", update_req.firmwareName); - printf(" Firmware Type : %s\n", update_req.TypeOfFirmware); - printf(" Location : %s\n", update_req.LocationOfFirmware); - printf(" Reboot Now : %s\n\n", update_req.rebootImmediately ? "true" : "false"); + EXAMPLE_INFO(" Firmware Name : %s\n", update_req.firmwareName); + EXAMPLE_INFO(" Firmware Type : %s\n", update_req.TypeOfFirmware); + EXAMPLE_INFO(" Location : %s\n", update_req.LocationOfFirmware); + EXAMPLE_INFO(" Reboot Now : %s\n", update_req.rebootImmediately ? "true" : "false"); - printf(" Calling updateFirmware()...\n\n"); + EXAMPLE_INFO(" Calling updateFirmware()...\n"); UpdateResult upd_result = updateFirmware(g_handle, &update_req, on_update_progress_callback); if (upd_result != RDKFW_UPDATE_SUCCESS) { - fprintf(stderr, "[ERROR] updateFirmware() returned FAIL!\n\n"); + EXAMPLE_ERROR("updateFirmware() returned FAIL!\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } - printf(" ✓ updateFirmware() returned SUCCESS\n"); - printf(" Waiting for flash progress...\n\n"); - printf(" Flash Progress:\n"); + EXAMPLE_INFO("updateFirmware() returned SUCCESS\n"); + EXAMPLE_INFO(" Waiting for flash progress...\n"); /* Wait for flash completion with timeout (10 minutes) */ clock_gettime(CLOCK_REALTIME, &timeout); @@ -535,7 +537,7 @@ int main(void) rc = pthread_cond_timedwait(&g_update_cond, &g_update_mutex, &timeout); if (rc != 0) { pthread_mutex_unlock(&g_update_mutex); - fprintf(stderr, "[ERROR] Timeout waiting for flash completion (10 min)\n\n"); + EXAMPLE_ERROR("Timeout waiting for flash completion (10 min)\n"); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } @@ -544,42 +546,41 @@ int main(void) /* Check flash result */ if (g_update_status != UPDATE_COMPLETED) { - fprintf(stderr, "[ERROR] Firmware flash failed (status=%d)\n\n", g_update_status); + EXAMPLE_ERROR("Firmware flash failed (status=%d)\n", g_update_status); g_exit_code = EXIT_FAILURE; goto cleanup_unregister; } - printf(" → Flash complete!\n\n"); + EXAMPLE_INFO("Flash complete!\n"); /* ==================================================================== * STEP 5: Unregister and Cleanup * ==================================================================== */ cleanup_unregister: - printf("│ STEP 5: Unregister from daemon │\n"); + EXAMPLE_INFO("STEP 5: Unregister from daemon\n"); if (g_handle != NULL) { - printf(" Calling unregisterProcess()...\n"); + EXAMPLE_INFO(" Calling unregisterProcess()...\n"); unregisterProcess(g_handle); g_handle = NULL; - printf(" ✓ Unregistered successfully\n\n"); + EXAMPLE_INFO(" Unregistered successfully\n"); } /* ==================================================================== * Final Status * ==================================================================== */ if (g_exit_code == EXIT_SUCCESS) { - printf("║ ✓ FIRMWARE UPDATE WORKFLOW COMPLETED ║\n"); + EXAMPLE_INFO("FIRMWARE UPDATE WORKFLOW COMPLETED\n"); if (g_update_status == UPDATE_COMPLETED) { - printf(" ⚠ NOTE: Firmware flashed successfully.\n"); - printf(" System reboot required to activate new firmware.\n"); - printf(" Use: systemctl reboot\n\n"); + EXAMPLE_INFO(" Firmware flashed successfully.\n"); + EXAMPLE_INFO(" System reboot required to activate new firmware.\n"); } } else { - printf("║ ✗ FIRMWARE UPDATE WORKFLOW FAILED ║\n"); - printf(" Check logs for details:\n"); - printf(" tail -f /opt/logs/rdkFwupdateMgr.log\n\n"); + EXAMPLE_ERROR("FIRMWARE UPDATE WORKFLOW FAILED\n"); + EXAMPLE_INFO(" Check logs for details: tail -f /opt/logs/rdkFwupdateMgr.log\n"); } + log_exit(); return g_exit_code; } diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_api.c b/librdkFwupdateMgr/src/rdkFwupdateMgr_api.c index 261af3ce..eda8d15a 100644 --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_api.c +++ b/librdkFwupdateMgr/src/rdkFwupdateMgr_api.c @@ -49,37 +49,132 @@ #include #include -/* ======================================================================== - * checkForUpdate — SYNCHRONOUS implementation - * ======================================================================== */ - -/** - * @brief Check for firmware update — non-blocking, returns immediately +/* + * checkForUpdate - Initiate a non-blocking firmware availability check. + * + * OVERVIEW + * + * PURPOSE: + * This is the FIRST async API call in the firmware update workflow. + * After registerProcess() gives you a handle, you call this to ask + * the daemon "is there a new firmware version for this device?" + * + * This function is NON-BLOCKING. It returns immediately (in ~3ms) + * with CHECK_FOR_UPDATE_SUCCESS, meaning "your request was accepted." + * The actual answer (FIRMWARE_AVAILABLE or FIRMWARE_NOT_AVAILABLE) + * arrives later (5-30 seconds) via your callback function, which is + * invoked by the library's background thread. + * + * WHAT "FIRE-AND-FORGET" MEANS: + * The D-Bus call to the daemon is fire-and-forget: + * - We send the message and do NOT wait for a reply + * - The daemon's method response is silently discarded (we already + * closed our ephemeral D-Bus connection by then) + * - The real answer comes as a BROADCAST D-Bus signal: + * "CheckForUpdateComplete" -- caught by the BG thread + * + * Think of it like mailing a letter: you drop it in the mailbox + * (return SUCCESS) and walk away. The reply comes later by separate + * delivery (the callback). + * + * THREADING MODEL: + * - This function runs on the CALLER'S thread (main thread) + * - It does NOT block the caller + * - The callback fires on the BACKGROUND thread (created during + * registerProcess -> internal_system_init) + * - The caller typically sleeps on a condvar until the callback + * sets a flag and signals it * - * Sends CheckForUpdate(handle) to the daemon and returns immediately. - * The daemon will query the XConf server in the background (5-30 seconds) - * and emit a CheckForUpdateComplete signal when done. + * D-BUS WIRE PROTOCOL: + * Method: "CheckForUpdate" + * Input: GVariant type "(s)" -- one string: the handle (e.g., "1") + * Reply: IGNORED (fire-and-forget -- three trailing NULLs) + * Signal: "CheckForUpdateComplete" type "(tiissss)" + * t handler_id (uint64) + * i result_code (int32) + * i status_code (int32: 0=available, 1=not available, 3=error) + * s current_version (e.g., "RDKV_7.0") + * s available_version (e.g., "RDKV_8.0") + * s update_details (pipe-separated "Key:Value|Key:Value|...") + * s status_message (human-readable) * - * The callback fires ONCE when the signal arrives with complete firmware info: - * - FwInfoData.status: FIRMWARE_AVAILABLE, FIRMWARE_NOT_AVAILABLE, etc. - * - FwInfoData.CurrFWVersion: Current firmware version - * - FwInfoData.UpdateDetails: Details about available update (if any) + * CONNECTION MODEL: + * This creates an EPHEMERAL D-Bus connection (e.g., :1.142) that + * lives only for the duration of this function call. The BG thread + * has its own PERSISTENT connection (:1.141) for receiving signals. + * These are completely independent. * - * The callback is registered in the async registry before sending the D-Bus call - * to ensure the signal doesn't arrive before we're ready to receive it. + * CALLBACK CONTRACT: + * - Fires exactly ONCE per checkForUpdate() call + * - Fires on the BG thread, NOT the caller's thread + * - Receives a const FwInfoData* that is STACK-ALLOCATED in the + * dispatch function -- valid ONLY during the callback + * - If you need data after the callback returns, you MUST copy it + * (e.g., strncpy to your own buffers) + * - If the daemon crashes or the signal never arrives, the callback + * NEVER fires -- the caller should use a condvar timeout (120s) + * + * RETURN VALUES: + * CHECK_FOR_UPDATE_SUCCESS (0) -- Request sent. Callback will fire later. + * CHECK_FOR_UPDATE_FAIL (1) -- Request failed. Callback will NOT fire. + * IMPORTANT: SUCCESS does NOT mean firmware is available. It means + * the request was accepted. Actual availability comes in the callback. + * + * EXECUTION FLOW (step numbers match code comments below): + * + * [1] Validate handle and callback (reject NULL/empty) + * [2] Open ephemeral D-Bus connection (fail early if D-Bus is down) + * [3] Register callback in g_registry (mutex-protected) + * -- MUST happen BEFORE sending the D-Bus call to avoid race + * [4] Send fire-and-forget "CheckForUpdate" D-Bus method call + * [5] Close ephemeral connection, return SUCCESS + * + * [Later, 5-30 seconds -- on BG thread:] + * Daemon broadcasts "CheckForUpdateComplete" signal + * BG thread receives it in on_check_complete_signal() + * dispatch_all_pending() finds our PENDING slot, invokes our callback + * Slot is reset to IDLE after callback returns + * + * WHY REGISTER BEFORE SEND (Step 3 before Step 4): + * If we sent the D-Bus call FIRST and the daemon responded instantly + * (e.g., cached result), the BG thread would receive the signal before + * we registered the callback. dispatch_all_pending() would scan the + * registry, find zero PENDING entries, and discard the signal. + * Our callback would never fire. The app would hang on condvar forever. + * + * By registering FIRST, the callback is waiting in the registry before + * the daemon can possibly respond. Race condition eliminated. + * + * WHY CONNECT BEFORE REGISTER (Step 2 before Step 3): + * If we registered the callback FIRST and then D-Bus connection failed, + * we'd have a "ghost" PENDING entry that will never be dispatched + * (because the D-Bus call was never sent, so the signal will never + * arrive). The slot would stay PENDING forever, wasting 1 of 30 slots. + * + * By connecting FIRST, we know D-Bus is up before we touch the registry. + * If connection fails, we return FAIL with a clean registry. + * + * @param handle The handle returned by registerProcess(). Must be + * non-NULL and non-empty. e.g., "1" + * @param callback Function pointer to invoke when the daemon's signal + * arrives. Must be non-NULL. Signature: + * void callback(const FwInfoData *fwinfodata) * - * @param handle Valid FirmwareInterfaceHandle from registerProcess() - * @param callback Invoked when CheckForUpdateComplete signal arrives * @return CHECK_FOR_UPDATE_SUCCESS or CHECK_FOR_UPDATE_FAIL + * + * See also: on_check_complete_signal() -- BG thread signal handler + * See also: dispatch_all_pending() -- two-phase callback dispatch + * See also: internal_register_callback() -- registry slot allocation */ CheckForUpdateResult checkForUpdate(FirmwareInterfaceHandle handle, UpdateEventCallback callback) { - /* [1] Validate */ + /* Validate inputs */ if (handle == NULL || handle[0] == '\0') { FWUPMGR_ERROR("checkForUpdate: invalid handle (NULL or empty)\n"); return CHECK_FOR_UPDATE_FAIL; } + if (callback == NULL) { FWUPMGR_ERROR("checkForUpdate: callback is NULL\n"); return CHECK_FOR_UPDATE_FAIL; @@ -87,11 +182,7 @@ CheckForUpdateResult checkForUpdate(FirmwareInterfaceHandle handle, FWUPMGR_INFO("checkForUpdate: handle='%s'\n", handle); - /* [2] Connect to D-Bus FIRST before registering callback - * - * This prevents stale registry entries if D-Bus connection fails. - * We only register the callback if we can successfully send the request. - */ + /* Open ephemeral D-Bus connection (before registry to avoid ghost entries) */ GError *error = NULL; GDBusConnection *conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); @@ -102,26 +193,14 @@ CheckForUpdateResult checkForUpdate(FirmwareInterfaceHandle handle, return CHECK_FOR_UPDATE_FAIL; } - /* [3] Register callback AFTER D-Bus connection succeeds - * - * Register immediately before sending to avoid race condition where - * the daemon responds before we're ready to receive the signal. - */ + /* Register callback BEFORE D-Bus call to prevent signal race */ if (!internal_register_callback(handle, callback)) { FWUPMGR_ERROR("checkForUpdate: registry full, handle='%s'\n", handle); g_object_unref(conn); return CHECK_FOR_UPDATE_FAIL; } - /* [4] Fire-and-forget D-Bus CheckForUpdate method call - * - * Arguments: (s) - * s handle — identifies this app to the daemon - * - * Three trailing NULLs = fire and forget (no reply waited for). - * g_dbus_connection_call() returns immediately. - * Daemon will emit CheckForUpdateComplete signal when XConf query finishes. - */ + /* Fire-and-forget D-Bus call (3 trailing NULLs = no reply) */ FWUPMGR_INFO("checkForUpdate: calling CheckForUpdate on daemon, handle='%s'\n", handle); @@ -140,85 +219,158 @@ CheckForUpdateResult checkForUpdate(FirmwareInterfaceHandle handle, NULL /* user_data: none */ ); + /* Close ephemeral connection; message already in kernel socket buffer */ g_object_unref(conn); FWUPMGR_INFO("checkForUpdate: D-Bus call sent, returning SUCCESS. " "Callback will fire when CheckForUpdateComplete signal arrives. " "handle='%s'\n", handle); - /* [5] Return immediately — app is unblocked */ return CHECK_FOR_UPDATE_SUCCESS; } - -/* ======================================================================== - * LIBRARY LIFECYCLE - * ======================================================================== */ - -/** - * @brief Library constructor — auto-called when .so is loaded +/* + * downloadFirmware - Initiate a non-blocking firmware download. * - * Initializes the internal async engine (registry + background thread) - * before any app code runs. - */ -__attribute__((constructor)) -static void rdkFwupdateMgr_lib_init(void) -{ - FWUPMGR_INFO("=== rdkFwupdateMgr library loading ===\n"); - if (internal_system_init() != 0) { - FWUPMGR_ERROR("rdkFwupdateMgr_lib_init: internal_system_init FAILED\n"); - } - FWUPMGR_INFO("=== rdkFwupdateMgr library ready ===\n"); -} - -/** - * @brief Library destructor — auto-called when .so is unloaded + * OVERVIEW * - * Stops background thread and frees all resources cleanly. - */ -__attribute__((destructor)) -static void rdkFwupdateMgr_lib_deinit(void) -{ - FWUPMGR_INFO("=== rdkFwupdateMgr library unloading ===\n"); - internal_system_deinit(); - FWUPMGR_INFO("=== rdkFwupdateMgr library unloaded ===\n"); -} - -/* ======================================================================== - * DOWNLOAD FIRMWARE PUBLIC API - * ======================================================================== + * PURPOSE: + * This is the SECOND async API call in the firmware update workflow. + * After checkForUpdate() confirmed firmware IS available (status == + * FIRMWARE_AVAILABLE and you got the filename + URL from UpdateDetails), + * you call this to tell the daemon "start downloading that file." * - * Implements: - * DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, - * FwDwnlReq fwdwnlreq, - * DownloadCallback callback); - * - * FLOW: - * 1. Validate: handle not NULL/empty, firmwareName not empty, callback not NULL - * 2. Connect to D-Bus (fail early if connection fails) - * 3. Register callback in download registry (AFTER D-Bus connection succeeds) - * 4. Fire DownloadFirmware D-Bus method call to daemon (fire-and-forget) - * 5. Return RDKFW_DWNL_SUCCESS immediately - * - * [later — fires multiple times as download progresses] - * Daemon emits DownloadProgress(progress%, status) signal repeatedly - * → on_download_progress_signal() fires in background thread - * → dispatch_all_dwnl_active() calls every ACTIVE DownloadCallback - * → slot stays ACTIVE until DWNL_COMPLETED or DWNL_ERROR - * ======================================================================== */ - -/** - * @brief Initiate firmware download — non-blocking, returns immediately + * This function is NON-BLOCKING. It returns immediately (in ~3ms) with + * RDKFW_DWNL_SUCCESS, meaning "your request was accepted." The actual + * download progress (0%, 10%, 50%, 100%) arrives later -- REPEATEDLY -- + * via your callback function, which is invoked by the library's + * background thread each time the daemon emits a DownloadProgress signal. + * + * KEY DIFFERENCE FROM checkForUpdate(): + * checkForUpdate callback fires ONCE (one signal, one callback, done). + * downloadFirmware callback fires MANY TIMES (one per progress report). + * The registry slot stays ACTIVE across all progress signals and only + * goes IDLE when the download ends (COMPLETED or ERROR). + * + * WHAT "FIRE-AND-FORGET" MEANS (SAME PATTERN AS checkForUpdate): + * We send the D-Bus method call and do NOT wait for a reply. + * The daemon's method response is discarded (connection already closed). + * The real data comes as BROADCAST D-Bus signals: "DownloadProgress" + * -- caught by the BG thread's on_download_progress_signal() handler. + * + * THREADING MODEL: + * - This function runs on the CALLER'S thread (main thread) + * - It does NOT block the caller + * - The callback fires on the BACKGROUND thread (the one created + * during registerProcess -> internal_system_init) + * - The BG thread fires the callback MULTIPLE TIMES (once per signal) + * - The caller typically sleeps on a condvar until the callback + * sets g_download_done=1 on a terminal status (COMPLETED/ERROR) + * + * D-BUS WIRE PROTOCOL: + * Method: "DownloadFirmware" + * Input: GVariant type "(ssss)" -- four strings: + * s handle e.g., "1" (from registerProcess) + * s firmwareName e.g., "firmware_v8.bin" + * s downloadUrl e.g., "http://cdn.example.com/fw" or "" (use XConf) + * s TypeOfFirmware e.g., "PCI" or "PDRI" or "PERIPHERAL" + * Reply: IGNORED (fire-and-forget -- three trailing NULLs) + * + * Signal (arrives later, MULTIPLE times): + * Name: "DownloadProgress" + * GVariant type "(tsuss)": + * t handler_id (uint64 - which client) + * s firmware_name (string - filename being downloaded) + * u progress_percent (uint32 - 0 to 100) + * s status_string (string - "NOTSTARTED", "INPROGRESS", "COMPLETED", "ERROR") + * s message (string - human-readable message) + * + * CONNECTION MODEL: + * Creates an EPHEMERAL D-Bus connection (e.g., :1.143) that lives + * only for this function call. The BG thread has its own PERSISTENT + * connection (:1.141) for receiving signals. Completely independent. + * + * DOWNLOAD REGISTRY (g_dwnl_registry -- SEPARATE from g_registry): + * This API uses its OWN registry, independent from checkForUpdate's. + * g_dwnl_registry has its own mutex, its own 30 slots, its own + * state machine. The two registries never interfere with each other. + * + * Slot lifecycle: IDLE --> ACTIVE --> IDLE + * IDLE: Slot free, no callback registered. + * ACTIVE: Callback registered. Fires on EVERY DownloadProgress signal. + * Stays ACTIVE across multiple signals (0%, 10%, 50%...). + * IDLE: Reset when DWNL_COMPLETED or DWNL_ERROR is received. + * + * Compare with checkForUpdate's lifecycle: + * IDLE --> PENDING --> DISPATCHED --> IDLE (fires ONCE) + * Download has NO "DISPATCHED" state because the slot fires repeatedly. + * + * CALLBACK CONTRACT: + * - Fires MULTIPLE TIMES (once per DownloadProgress signal) + * - Fires on the BG thread, NOT the caller's thread + * - Signature: void callback(int progress_per, DownloadStatus status) + * - progress_per: 0 to 100 (percentage complete) + * - status: DWNL_IN_PROGRESS, DWNL_COMPLETED, or DWNL_ERROR + * - DWNL_COMPLETED means download finished successfully + * - DWNL_ERROR means download failed (network error, disk full, etc.) + * - After COMPLETED or ERROR, no more callbacks will fire + * - If daemon crashes mid-download, callback NEVER fires with + * COMPLETED/ERROR -- the caller's condvar timeout is the safety net + * + * RETURN VALUES: + * RDKFW_DWNL_SUCCESS (0) -- Request sent. Callbacks will fire later. + * RDKFW_DWNL_FAILED (1) -- Request failed. Callback will NOT fire. + * IMPORTANT: SUCCESS does NOT mean download started. It means the + * request was accepted. Actual progress comes in the callbacks. + * + * EXECUTION FLOW (step numbers match code comments below): + * + * [1] Validate handle, fwdwnlreq, firmwareName, callback (reject NULL/empty) + * [2] Open ephemeral D-Bus connection (fail early if D-Bus is down) + * [3] Register callback in g_dwnl_registry (state = ACTIVE) + * -- MUST happen BEFORE sending D-Bus call to avoid race + * [4] Send fire-and-forget "DownloadFirmware" D-Bus method call + * [5] Close ephemeral connection, return RDKFW_DWNL_SUCCESS + * + * [Later, repeatedly -- on BG thread:] + * Daemon broadcasts "DownloadProgress" signal (multiple times) + * BG thread receives it in on_download_progress_signal() + * dispatch_all_dwnl_active() finds our ACTIVE slot, invokes callback + * If status == COMPLETED or ERROR: slot is reset to IDLE + * Otherwise: slot stays ACTIVE for next signal + * + * WHY REGISTER BEFORE SEND (Step 3 before Step 4): + * Same race condition as checkForUpdate. If the daemon responds + * instantly (e.g., file already cached locally), the BG thread + * would receive the signal before we registered. The dispatch + * would find zero ACTIVE entries and silently drop the signal. + * Our callback would never fire. The app would hang forever. + * + * WHY CONNECT BEFORE REGISTER (Step 2 before Step 3): + * If we registered first and D-Bus connection then failed, we'd + * have a ghost ACTIVE entry that never fires (because the D-Bus + * call was never sent). The slot would stay ACTIVE forever, + * wasting 1 of 30 slots and never being cleaned up. + * + * @param handle The handle returned by registerProcess(). Must be + * non-NULL and non-empty. e.g., "1" + * @param fwdwnlreq Pointer to download request struct. Must be non-NULL. + * Contains firmwareName (required), downloadUrl (optional, + * "" means use XConf URL), TypeOfFirmware (optional). + * @param callback Function pointer invoked on each DownloadProgress signal. + * Must be non-NULL. Signature: + * void callback(int progress_per, DownloadStatus status) * - * @param handle Valid FirmwareInterfaceHandle from registerProcess() - * @param fwdwnlreq Download request (passed by value, library copies it) - * @param callback Invoked on each DownloadProgress signal * @return RDKFW_DWNL_SUCCESS or RDKFW_DWNL_FAILED + * + * See also: on_download_progress_signal() -- BG thread signal handler + * See also: dispatch_all_dwnl_active() -- two-phase callback dispatch + * See also: internal_dwnl_register_callback() -- registry slot allocation */ DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, const FwDwnlReq *fwdwnlreq, DownloadCallback callback) { - /* [1] Validate */ + /* Validate inputs */ if (handle == NULL || handle[0] == '\0') { FWUPMGR_ERROR("downloadFirmware: invalid handle (NULL or empty)\n"); return RDKFW_DWNL_FAILED; @@ -250,10 +402,7 @@ DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, (fwdwnlreq->TypeOfFirmware && fwdwnlreq->TypeOfFirmware[0]) ? fwdwnlreq->TypeOfFirmware : "(none)", (fwdwnlreq->downloadUrl && fwdwnlreq->downloadUrl[0]) ? fwdwnlreq->downloadUrl : "(use XConf)"); - /* [2] Connect to D-Bus FIRST before registering callback - * - * This prevents stale registry entries if D-Bus connection fails. - */ + /* Open ephemeral D-Bus connection (before registry to avoid ghost entries) */ GError *error = NULL; GDBusConnection *conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); @@ -264,28 +413,14 @@ DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, return RDKFW_DWNL_FAILED; } - /* [3] Register callback AFTER D-Bus connection succeeds, BEFORE sending - * - * Register immediately before sending to avoid race condition where - * the daemon responds before we're ready to receive the signal. - */ + /* Register callback BEFORE D-Bus call to prevent signal race */ if (!internal_dwnl_register_callback(handle, callback)) { FWUPMGR_ERROR("downloadFirmware: registry full, handle='%s'\n", handle); g_object_unref(conn); return RDKFW_DWNL_FAILED; } - /* [4] Fire-and-forget D-Bus DownloadFirmware method call - * - * Arguments: (ssss) - * s handle — identifies this app to the daemon - * s firmwareName — firmware image filename - * s downloadUrl — override URL or "" for XConf URL - * s TypeOfFirmware — "PCI" | "PDRI" | "PERIPHERAL" - * - * Three trailing NULLs = fire and forget (no reply waited for). - * g_dbus_connection_call() returns immediately. - */ + /* Fire-and-forget D-Bus call with 4 string args (handle, name, url, type) */ g_dbus_connection_call( conn, @@ -306,12 +441,12 @@ DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, NULL /* user_data: none */ ); + /* Close ephemeral connection; message already in kernel socket buffer */ g_object_unref(conn); FWUPMGR_INFO("downloadFirmware: D-Bus call sent, returning SUCCESS. handle='%s'\n", handle); - /* [4] Return immediately — app is unblocked */ return RDKFW_DWNL_SUCCESS; } @@ -319,46 +454,117 @@ DownloadResult downloadFirmware(FirmwareInterfaceHandle handle, * UPDATE FIRMWARE PUBLIC API * ======================================================================== * + * updateFirmware -- Initiate firmware flashing (non-blocking) + * * Implements: * UpdateResult updateFirmware(FirmwareInterfaceHandle handle, - * FwUpdateReq fwupdatereq, + * const FwUpdateReq *fwupdatereq, * UpdateCallback callback); * - * FLOW: - * 1. Validate: handle not NULL/empty, firmwareName not empty, - * TypeOfFirmware not empty, callback not NULL - * 2. Connect to D-Bus (fail early if connection fails) - * 3. Register callback in update registry (AFTER D-Bus connection succeeds) - * 4. Fire UpdateFirmware D-Bus method call to daemon (fire-and-forget) - * 5. Return RDKFW_UPDATE_SUCCESS immediately - * - * [later — fires multiple times as flashing progresses] - * Daemon emits UpdateProgress(progress%, status) signal repeatedly - * → on_update_progress_signal() fires in background thread - * → dispatch_all_update_active() calls every ACTIVE UpdateCallback - * → slot stays ACTIVE until UPDATE_COMPLETED or UPDATE_ERROR - * ======================================================================== */ - -/** - * @brief Initiate firmware flashing — non-blocking, returns immediately - * - * D-Bus arguments sent to daemon: (sssss) - * s handle — identifies this app - * s firmwareName — image filename to flash - * s LocationOfFirmware — path to image ("" = use device.properties) - * s TypeOfFirmware — "PCI" | "PDRI" | "PERIPHERAL" - * s rebootImmediately — "true" or "false" (daemon expects string) - * - * @param handle Valid FirmwareInterfaceHandle from registerProcess() - * @param fwupdatereq Update request (passed by value, library copies it) - * @param callback Invoked on each UpdateProgress signal + * PURPOSE: + * This is the third and final step in the firmware lifecycle: + * 1. checkForUpdate() -- ask the daemon if new firmware exists + * 2. downloadFirmware() -- download the firmware image + * 3. updateFirmware() -- flash the downloaded image onto the device + * + * updateFirmware() sends a fire-and-forget D-Bus method call to the + * daemon instructing it to flash the specified firmware image. The + * function returns IMMEDIATELY -- the actual flashing happens on the + * daemon side and may take minutes. Progress is delivered through + * repeated callbacks on the BG thread. + * + * RETURN VALUES: + * RDKFW_UPDATE_SUCCESS (0) -- Request sent. Callbacks will fire later. + * RDKFW_UPDATE_FAILED (1) -- Request failed. Callback will NOT fire. + * IMPORTANT: SUCCESS does NOT mean flashing started. It means the + * request was accepted. Actual progress comes in the callbacks. + * + * CALLBACK CONTRACT: + * The UpdateCallback is invoked MULTIPLE TIMES (like downloadFirmware, + * unlike checkForUpdate which fires once). Each invocation carries: + * int progress_per -- 0 to 100 (percent complete) + * UpdateStatus status -- UPDATE_IN_PROGRESS, UPDATE_COMPLETED, or + * UPDATE_ERROR + * The callback fires on the BG thread (NOT the main thread). If the + * app needs to update UI, it must marshal the call to the main thread. + * + * EXECUTION FLOW (step numbers match code comments below): + * + * [1] Validate handle, fwupdatereq, firmwareName, TypeOfFirmware, + * callback (reject NULL/empty). This has the MOST validations + * of all three APIs (7 checks vs 4 for checkForUpdate, 5 for + * downloadFirmware) because FwUpdateReq has more required fields. + * [2] Open ephemeral D-Bus connection (fail early if D-Bus is down) + * [3] Register callback in g_update_registry (state = ACTIVE) + * -- MUST happen BEFORE sending D-Bus call to avoid race + * [4] Send fire-and-forget "UpdateFirmware" D-Bus method call + * [5] Close ephemeral connection, return RDKFW_UPDATE_SUCCESS + * + * [Later, repeatedly -- on BG thread:] + * Daemon broadcasts "UpdateProgress" signal (multiple times) + * BG thread receives it in on_update_progress_signal() + * dispatch_all_update_active() finds our ACTIVE slot, invokes callback + * If status == UPDATE_COMPLETED or UPDATE_ERROR: slot is reset to IDLE + * Otherwise: slot stays ACTIVE for next signal + * + * WHY REGISTER BEFORE SEND (Step 3 before Step 4): + * Same race condition as checkForUpdate and downloadFirmware. If the + * daemon responds instantly (e.g., trivial flash operation), the BG + * thread would receive the signal before we registered. The dispatch + * would find zero ACTIVE entries and silently drop the signal. + * Our callback would never fire. The app would hang forever. + * + * WHY CONNECT BEFORE REGISTER (Step 2 before Step 3): + * If we registered first and D-Bus connection then failed, we'd + * have a ghost ACTIVE entry that never fires (because the D-Bus + * call was never sent). The slot would stay ACTIVE forever, + * wasting 1 of 30 slots and never being cleaned up. + * + * D-BUS ARGUMENTS: (sssss) -- five strings + * This is the ONLY API that sends 5 strings. For comparison: + * checkForUpdate: (s) -- 1 string (handle) + * downloadFirmware: (ssss) -- 4 strings (handle, name, url, type) + * updateFirmware: (sssss) -- 5 strings (handle, name, location, + * type, rebootImmediately) + * + * The 5th argument, rebootImmediately, is a boolean in FwUpdateReq + * but is sent as a string "true"/"false" because the daemon's D-Bus + * interface expects all arguments as strings. + * + * REGISTRY DIFFERENCES FROM checkForUpdate AND downloadFirmware: + * - Uses g_update_registry (third separate registry, not g_registry + * or g_dwnl_registry) + * - Slot state = UPDATE_CB_STATE_ACTIVE (fires repeatedly, like download) + * - Signal format = "(tsiis)" not "(tsuss)" like download + * t = handler_id (uint64), s = firmware_name, i = progress (int32), + * i = status_code (int32), s = message + * - Status mapping uses INTEGER codes (0=IN_PROGRESS, 1=COMPLETED, + * 2=ERROR) via internal_map_update_status_code(), not STRING codes + * like download's map_dwnl_status_string() + * + * @param handle The handle returned by registerProcess(). Must be + * non-NULL and non-empty. e.g., "1" + * @param fwupdatereq Pointer to update request struct. Must be non-NULL. + * Contains firmwareName (required), LocationOfFirmware + * (optional, "" means use device.properties path), + * TypeOfFirmware (required, e.g. "PCI"), + * rebootImmediately (bool, converted to "true"/"false"). + * @param callback Function pointer invoked on each UpdateProgress signal. + * Must be non-NULL. Signature: + * void callback(int progress_per, UpdateStatus status) + * * @return RDKFW_UPDATE_SUCCESS or RDKFW_UPDATE_FAILED - */ + * + * See also: on_update_progress_signal() -- BG thread signal handler + * See also: dispatch_all_update_active() -- two-phase callback dispatch + * See also: internal_update_register_callback() -- registry slot allocation + * See also: internal_map_update_status_code() -- integer->enum mapping + * ======================================================================== */ UpdateResult updateFirmware(FirmwareInterfaceHandle handle, const FwUpdateReq *fwupdatereq, UpdateCallback callback) { - /* [1] Validate */ + /* Validate inputs (most checks of all 3 APIs -- TypeOfFirmware required) */ if (handle == NULL || handle[0] == '\0') { FWUPMGR_ERROR("updateFirmware: invalid handle (NULL or empty)\n"); return RDKFW_UPDATE_FAILED; @@ -404,10 +610,7 @@ UpdateResult updateFirmware(FirmwareInterfaceHandle handle, : "(use device.properties path)", fwupdatereq->rebootImmediately ? "yes" : "no"); - /* [2] Connect to D-Bus FIRST before registering callback - * - * This prevents stale registry entries if D-Bus connection fails. - */ + /* Open ephemeral D-Bus connection (before registry to avoid ghost entries) */ GError *error = NULL; GDBusConnection *conn = g_bus_get_sync(G_BUS_TYPE_SYSTEM, NULL, &error); @@ -418,28 +621,14 @@ UpdateResult updateFirmware(FirmwareInterfaceHandle handle, return RDKFW_UPDATE_FAILED; } - /* [3] Register callback AFTER D-Bus connection succeeds, BEFORE sending - * - * Register immediately before sending to avoid race condition where - * the daemon responds before we're ready to receive the signal. - */ + /*Register callback BEFORE D-Bus call to prevent signal race */ if (!internal_update_register_callback(handle, callback)) { FWUPMGR_ERROR("updateFirmware: registry full, handle='%s'\n", handle); g_object_unref(conn); return RDKFW_UPDATE_FAILED; } - /* [4] Fire-and-forget D-Bus UpdateFirmware method call - * - * Arguments: (sssss) - * s handle — app's handler_id string - * s firmwareName — image to flash - * s LocationOfFirmware — path or "" for device.properties default - * s TypeOfFirmware — PCI / PDRI / PERIPHERAL - * s rebootImmediately — "true" or "false" (daemon expects string) - * - * Three trailing NULLs = fire and forget. - */ + /* Fire-and-forget D-Bus call with 5 string args (handle, name, location, type, reboot) */ g_dbus_connection_call( conn, @@ -447,7 +636,7 @@ UpdateResult updateFirmware(FirmwareInterfaceHandle handle, DBUS_OBJECT_PATH, DBUS_INTERFACE_NAME, DBUS_METHOD_UPDATE, /* method: UpdateFirmware */ - g_variant_new("(sssss)", /* ✅ 5 strings now! */ + g_variant_new("(sssss)", /* 5 strings (see above) */ handle, /* app's handler_id string */ fwupdatereq->firmwareName, /* image to flash */ fwupdatereq->LocationOfFirmware ? fwupdatereq->LocationOfFirmware : "", /* path or "" */ @@ -461,11 +650,11 @@ UpdateResult updateFirmware(FirmwareInterfaceHandle handle, NULL /* user_data: none */ ); + /* Close ephemeral connection; message already in kernel socket buffer */ g_object_unref(conn); FWUPMGR_INFO("updateFirmware: D-Bus call sent, returning SUCCESS. " "handle='%s'\n", handle); - /* [4] Return immediately — app is unblocked */ return RDKFW_UPDATE_SUCCESS; } diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_async.c b/librdkFwupdateMgr/src/rdkFwupdateMgr_async.c index f1ffcc46..3bfb25ab 100644 --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_async.c +++ b/librdkFwupdateMgr/src/rdkFwupdateMgr_async.c @@ -330,14 +330,66 @@ static void *background_thread_func(void *arg) * D-BUS SIGNAL HANDLER * ======================================================================== */ -/** - * @brief Called by GLib when CheckForUpdateComplete signal arrives +/* + * on_check_complete_signal - D-Bus signal handler for CheckForUpdateComplete. + * + * OVERVIEW + * + * PURPOSE: + * This function is called by GLib's D-Bus infrastructure when the daemon + * broadcasts the "CheckForUpdateComplete" signal. It is the ENTRY POINT + * for the "response" side of the checkForUpdate() async flow. + * + * WHEN DOES THIS FIRE? + * 5-30 seconds after checkForUpdate() was called. The daemon queried + * the XConf cloud server for firmware availability, got a response, + * and broadcast the result as a D-Bus signal to ALL listeners. + * + * WHICH THREAD RUNS THIS? + * The BACKGROUND THREAD. Not the main thread. This is critical. + * + * The BG thread is blocked in g_main_loop_run() waiting for events. + * When the signal arrives on the BG thread's persistent D-Bus connection + * (:1.141), GLib wakes the BG thread and dispatches to this handler. + * This handler was registered via g_dbus_connection_signal_subscribe() + * in background_thread_func() during registerProcess(). + * + * PARAMETERS: + * conn -- the BG thread's persistent D-Bus connection (:1.141) + * sender -- the daemon's unique sender name (e.g., ":1.5") + * object_path -- "/org/rdkfwupdater/Service" + * interface_name -- "org.rdkfwupdater.Interface" + * signal_name -- "CheckForUpdateComplete" + * parameters -- GVariant of type "(tiissss)" containing the result + * user_data -- NULL (we use global state, not user_data) * - * Runs in the background thread context. + * All parameters except 'parameters' are unused (cast to void). + * We only care about the GVariant payload. * - * 1. Parse GVariant payload → InternalSignalData - * 2. Dispatch to all PENDING registry entries - * 3. Free parsed signal data + * EXECUTION FLOW: + * 1. Parse the GVariant "(tiissss)" into an InternalSignalData struct + * (4 strdup'd strings: current_version, available_version, + * update_details, status_message) + * 2. Call dispatch_all_pending() which: + * a. Finds all PENDING registry entries + * b. Builds FwInfoData from the signal data + * c. Invokes each callback + * d. Resets each slot to IDLE + * 3. Free the 4 strdup'd strings via internal_cleanup_signal_data() + * + * ERROR HANDLING: + * If GVariant parsing fails (wrong type signature, corrupt data), + * we log an error and return without dispatching. Callbacks will + * NOT fire. The caller's condvar timeout will eventually expire. + * + * MEMORY: + * internal_parse_signal_data() allocates 4 strings via strdup(). + * internal_cleanup_signal_data() frees them after dispatch completes. + * The InternalSignalData struct itself is on the stack (this function's + * stack frame on the BG thread). + * + * After this function returns, the BG thread goes back to + * g_main_loop_run() and sleeps until the next signal. */ static void on_check_complete_signal(GDBusConnection *conn, const gchar *sender, @@ -347,11 +399,13 @@ static void on_check_complete_signal(GDBusConnection *conn, GVariant *parameters, gpointer user_data) { + /* Suppress unused parameter warnings (GLib signal handler requires all 7) */ (void)conn; (void)sender; (void)object_path; (void)interface_name; (void)signal_name; (void)user_data; FWUPMGR_INFO("on_check_complete_signal: received\n"); + /* Parse signal payload "(tiissss)" into local struct */ InternalSignalData signal_data; memset(&signal_data, 0, sizeof(signal_data)); @@ -360,36 +414,95 @@ static void on_check_complete_signal(GDBusConnection *conn, return; } + /* Dispatch to all registered callbacks, then free strdup'd strings */ dispatch_all_pending(&signal_data); - internal_cleanup_signal_data(&signal_data); } -/** - * @brief Dispatch signal result to every PENDING callback +/* + * dispatch_all_pending - Find all PENDING callbacks and invoke them. + * + * OVERVIEW + * + * PURPOSE: + * This is the CORE of the async engine. When a CheckForUpdateComplete + * signal arrives, this function finds every PENDING callback in the + * registry, builds the FwInfoData struct from the signal payload, + * and invokes each callback. * - * TWO-PHASE DESIGN — avoids deadlock: + * WHY "ALL PENDING" (NOT JUST ONE)? + * If multiple clients (or the same client calling checkForUpdate + * multiple times) have PENDING entries, they all get the same + * firmware check result. The daemon broadcasts ONE signal and + * ALL pending callbacks receive it. This is the "fan-out" pattern. * - * PHASE 1 (mutex held): - * Scan registry → snapshot all PENDING entries into local array. - * Mark each found entry as DISPATCHED. - * Release mutex. + * In the typical single-client case, there is exactly 1 PENDING entry. * - * PHASE 2 (mutex released): - * Build FwUpdateEventData from signal_data. - * Invoke each snapshot callback: callback(handle, &event_data) - * Re-acquire mutex briefly to reset each slot to IDLE. + * TWO-PHASE DESIGN -- THE DEADLOCK PREVENTION PATTERN: * - * WHY RELEASE BEFORE CALLING CALLBACKS? - * If a callback called checkForUpdate() again, it would call - * internal_register_callback() which tries to lock the same mutex - * → deadlock. Releasing first makes re-entrant use safe. + * PHASE 1 (mutex HELD): + * Lock g_registry.mutex. + * Scan all 30 entries. For each PENDING entry: + * - Copy its callback pointer and handle into a local stack array + * - Change its state from PENDING to DISPATCHED + * Unlock g_registry.mutex. * - * @param signal_data Parsed signal payload (shared across all callbacks) + * PHASE 2 (mutex RELEASED): + * Build FwInfoData from signal_data (stack-allocated). + * For each entry in the snapshot: + * - Invoke: callback(&fwinfo_data) + * - After callback returns: lock mutex, reset slot to IDLE, unlock + * + * WHY NOT HOLD THE MUTEX DURING CALLBACK INVOCATION? + * + * Scenario that would deadlock WITHOUT two-phase: + * 1. BG thread holds g_registry.mutex + * 2. BG thread calls callback(&fwinfo_data) + * 3. Inside the callback, the app calls checkForUpdate() again + * (re-entrant use -- not common, but must be safe) + * 4. checkForUpdate() calls internal_register_callback() + * 5. internal_register_callback() calls pthread_mutex_lock(&g_registry.mutex) + * 6. DEADLOCK -- the BG thread is already holding that mutex + * (from step 1), and it's the same thread trying to re-acquire it + * + * With two-phase, the mutex is released BEFORE step 2, so step 5 + * would succeed (no one holds the mutex). + * + * Even if the callback does NOT call checkForUpdate() again, holding + * the mutex during a potentially slow callback (imagine the callback + * does heavy work -- file I/O, network, etc.) would block the main + * thread from registering new callbacks until the slow callback finishes. + * Two-phase keeps the critical section (Phase 1) fast: just a scan + * and copy, microseconds. + * + * WHY MARK AS DISPATCHED (NOT JUST SKIP IDLE)? + * DISPATCHED is an intermediate state between PENDING and IDLE. + * It means "we're about to call this callback but haven't finished yet." + * If another signal arrives while Phase 2 is running (very unlikely but + * possible), the next dispatch_all_pending() call would see DISPATCHED + * and skip it -- preventing double-dispatch of the same callback. + * + * MEMORY MODEL: + * - The Snapshot struct is stack-allocated (local array of 30 entries) + * - Each Snapshot copies the callback pointer and handle string + * - FwInfoData is stack-allocated in this function's frame + * - UpdateDetails is stack-allocated in this function's frame + * - ALL of this data is valid ONLY during callback execution + * - When this function returns, all stack data is gone + * - The callback MUST copy any data it needs before returning + * + * THREAD: Always runs on the BG thread (called from on_check_complete_signal). + * + * @param signal_data Parsed signal payload from internal_parse_signal_data(). + * Contains strdup'd strings -- valid until cleanup. + * + * Called by: on_check_complete_signal() + * Calls: internal_map_status_code(), parse_update_details(), + * each registered callback, registry_reset_slot() */ static void dispatch_all_pending(const InternalSignalData *signal_data) { - /* Local snapshot — avoids holding mutex during callback invocations */ + /* Stack-local snapshot: copied from each PENDING entry while mutex is held */ typedef struct { UpdateEventCallback callback; char handle_copy[256]; @@ -399,19 +512,21 @@ static void dispatch_all_pending(const InternalSignalData *signal_data) Snapshot snapshots[MAX_PENDING_CALLBACKS]; int count = 0; - /* ---- PHASE 1: collect under mutex ---- */ + /* PHASE 1: Collect PENDING entries under mutex (fast critical section) */ pthread_mutex_lock(&g_registry.mutex); for (int i = 0; i < MAX_PENDING_CALLBACKS; i++) { CallbackEntry *e = &g_registry.entries[i]; if (e->state != CB_STATE_PENDING) continue; + /* Copy essential data into snapshot for mutex-free invocation */ snapshots[count].callback = e->callback; snapshots[count].slot_index = i; snprintf(snapshots[count].handle_copy, sizeof(snapshots[count].handle_copy), "%s", e->handle_key ? e->handle_key : ""); + /* Mark DISPATCHED to prevent double-dispatch or re-entrant overwrite */ e->state = CB_STATE_DISPATCHED; count++; @@ -419,76 +534,59 @@ static void dispatch_all_pending(const InternalSignalData *signal_data) e->handle_key ? e->handle_key : "(null)"); } + /* Release mutex; main thread can now register new callbacks */ pthread_mutex_unlock(&g_registry.mutex); FWUPMGR_INFO("dispatch_all_pending: %d callback(s) to fire\n", count); - /* ---- PHASE 2: invoke callbacks, no mutex held ---- */ + /* PHASE 2: Build FwInfoData and invoke callbacks (no mutex held) */ + /* Map daemon's integer status_code to public enum */ CheckForUpdateStatus status = internal_map_status_code(signal_data->status_code); - /* - * Build FwInfoData with UpdateDetails for the callback. - * This matches the public API signature: UpdateEventCallback(const FwInfoData*) - * - * MEMORY MANAGEMENT: - * - FwInfoData is stack-allocated (valid during callback invocations) - * - CurrFWVersion is copied from signal_data (array, not pointer) - * - UpdateDetails is stack-allocated if needed - * - All data valid until end of this function - */ + /* Build stack-allocated FwInfoData (valid only during callbacks) */ FwInfoData fwinfo_data; memset(&fwinfo_data, 0, sizeof(fwinfo_data)); - /* Copy current firmware version */ + /* Copy current version from signal into fixed-size buffer */ if (signal_data->current_version) { strncpy(fwinfo_data.CurrFWVersion, signal_data->current_version, sizeof(fwinfo_data.CurrFWVersion) - 1); fwinfo_data.CurrFWVersion[sizeof(fwinfo_data.CurrFWVersion) - 1] = '\0'; } - /* Set status */ + /* Set the status enum in the struct. */ fwinfo_data.status = status; - /* Parse and populate UpdateDetails if firmware is available */ + /* Parse UpdateDetails from pipe-separated string if firmware is available */ UpdateDetails update_details; if (status == FIRMWARE_AVAILABLE && signal_data->update_details) { memset(&update_details, 0, sizeof(update_details)); if (parse_update_details(signal_data->update_details, &update_details)) { - /* Point FwInfoData to our stack-allocated UpdateDetails */ fwinfo_data.UpdateDetails = &update_details; FWUPMGR_INFO("dispatch_all_pending: UpdateDetails populated\n"); FWUPMGR_INFO(" FwFileName: %s\n", update_details.FwFileName); FWUPMGR_INFO(" FwVersion: %s\n", update_details.FwVersion); } else { - /* Parse failed - set to NULL to indicate no details available */ fwinfo_data.UpdateDetails = NULL; FWUPMGR_ERROR("dispatch_all_pending: parse_update_details failed\n"); } } else { - /* Status is not FIRMWARE_AVAILABLE or no update_details string */ fwinfo_data.UpdateDetails = NULL; } - /* Invoke all callbacks with the same FwInfoData */ + /* Invoke each callback sequentially, then reset slot to IDLE */ for (int i = 0; i < count; i++) { Snapshot *s = &snapshots[i]; FWUPMGR_INFO("dispatch_all_pending: invoking callback for handle='%s'\n", s->handle_copy); - /* - * Invoke callback with proper signature: - * UpdateEventCallback(const FwInfoData *fwinfodata) - * - * handle_copy is passed but callback signature doesn't use it anymore. - * We pass it to maintain compatibility with 2-param callbacks if needed. - */ s->callback(&fwinfo_data); - /* Reset slot to IDLE */ + /* Reset slot to IDLE under mutex (frees strdup'd handle_key) */ pthread_mutex_lock(&g_registry.mutex); registry_reset_slot(&g_registry.entries[s->slot_index]); pthread_mutex_unlock(&g_registry.mutex); @@ -499,29 +597,77 @@ static void dispatch_all_pending(const InternalSignalData *signal_data) * REGISTRY OPERATIONS * ======================================================================== */ -/** - * @brief Register a pending callback keyed by handle (no user_data) +/* + * internal_register_callback - Store a callback in the check-for-update registry. * - * SAME HANDLE TWICE: - * If the same handle is still PENDING from a previous call, its slot - * is overwritten. Prevents ghost callbacks accumulating. + * OVERVIEW + * + * PURPOSE: + * Called by checkForUpdate() to record the caller's callback function + * in g_registry so that when the BG thread later receives the + * CheckForUpdateComplete D-Bus signal, it can find and invoke it. + * + * Think of it as writing your name and phone number on a waiting list. + * When the result arrives, the BG thread walks the list and calls + * everyone who signed up. + * + * REGISTRY STRUCTURE: + * g_registry is a static global CallbackRegistry: + * - entries[30] -- array of CallbackEntry structs (MAX_PENDING_CALLBACKS=30) + * - mutex -- pthread_mutex_t protecting the array + * - initialized -- bool (set by internal_system_init) + * + * Each CallbackEntry has: + * - state -- IDLE, PENDING, DISPATCHED, or TIMED_OUT + * - handle_key -- strdup'd copy of the handle string (e.g., "1") + * - callback -- function pointer to the caller's callback + * - registered_time -- unix timestamp for potential timeout detection + * + * SLOT LIFECYCLE: + * IDLE -- slot is empty, available for use + * PENDING -- callback registered, waiting for signal from daemon + * DISPATCHED -- signal received, callback is being invoked right now + * IDLE -- callback returned, slot reset and available again * - * @param handle App's FirmwareInterfaceHandle (will be strdup'd) - * @param callback App's 2-param UpdateEventCallback - * @return true on success, false if registry is full + * DEDUP BEHAVIOR: + * If the same handle already has a PENDING entry (the caller called + * checkForUpdate() twice before the first callback fired), the old + * entry is OVERWRITTEN with the new callback. This prevents "ghost" + * callbacks from accumulating. The old callback will never fire. + * + * THREAD SAFETY: + * Protected by g_registry.mutex. The main thread calls this function + * (to register). The BG thread calls dispatch_all_pending() (to read + * and dispatch). The mutex ensures they never see inconsistent state. + * + * MEMORY: + * handle_key is strdup'd here (heap allocation). It is freed either: + * a. When the slot is reset to IDLE (registry_reset_slot) + * b. When an existing entry is overwritten (dedup path) + * c. When internal_system_deinit cleans up all remaining entries + * + * @param handle The client's handle string (e.g., "1"). Will be + * strdup'd -- caller retains ownership of their copy. + * @param callback The function to call when the signal arrives. + * Signature: void callback(const FwInfoData *fwinfodata) + * @return true if registered successfully, false if registry is full (30 slots) + * + * Called by: checkForUpdate() in rdkFwupdateMgr_api.c + * Pairs with: dispatch_all_pending() which reads PENDING entries */ bool internal_register_callback(FirmwareInterfaceHandle handle, UpdateEventCallback callback) { pthread_mutex_lock(&g_registry.mutex); + /* Single-pass scan: look for dedup match (same handle) and first free slot */ CallbackEntry *free_slot = NULL; CallbackEntry *existing_slot = NULL; for (int i = 0; i < MAX_PENDING_CALLBACKS; i++) { CallbackEntry *e = &g_registry.entries[i]; - /* Existing pending entry for same handle → overwrite it */ + /* Dedup: overwrite existing PENDING entry for same handle */ if (e->state == CB_STATE_PENDING && e->handle_key != NULL && strcmp(e->handle_key, handle) == 0) { @@ -534,6 +680,7 @@ bool internal_register_callback(FirmwareInterfaceHandle handle, } } + /* Prefer dedup (overwrite) over new slot */ CallbackEntry *target = existing_slot ? existing_slot : free_slot; if (target == NULL) { @@ -543,6 +690,7 @@ bool internal_register_callback(FirmwareInterfaceHandle handle, return false; } + /* If overwriting, free the old handle_key to avoid leak */ if (existing_slot) { FWUPMGR_INFO("internal_register_callback: overwriting existing for handle='%s'\n", handle); @@ -550,6 +698,7 @@ bool internal_register_callback(FirmwareInterfaceHandle handle, target->handle_key = NULL; } + /* Populate slot: strdup handle, store callback, mark PENDING */ target->handle_key = strdup(handle); target->callback = callback; target->state = CB_STATE_PENDING; @@ -561,9 +710,40 @@ bool internal_register_callback(FirmwareInterfaceHandle handle, return true; } -/** - * @brief Reset a registry slot to IDLE - * MUST be called with registry mutex held. +/* + * registry_reset_slot - Return a CallbackEntry to the IDLE state. + * + * PURPOSE: + * After a callback has been dispatched (invoked and returned), the + * registry slot must be cleaned up and made available for reuse. + * This function frees the strdup'd handle_key, clears the callback + * pointer, resets the timestamp, and sets state to IDLE. + * + * PRECONDITION: + * Caller MUST hold g_registry.mutex before calling this function. + * dispatch_all_pending() does this: lock -> reset -> unlock. + * internal_system_deinit() also calls this during cleanup. + * + * Why must the mutex be held? + * Without the mutex, a race could occur: + * - BG thread is resetting slot 0 (setting state to IDLE) + * - Main thread scans for free slots (sees IDLE in half-written state) + * - Main thread writes into slot 0 while BG thread is still clearing it + * The mutex ensures atomicity of the reset operation. + * + * MEMORY: + * handle_key was allocated by strdup() in internal_register_callback(). + * We free() it here. After this call, entry->handle_key is NULL. + * The callback function pointer is just zeroed (it points to code + * segment, not heap -- no need to free). + * + * STATE TRANSITION: + * DISPATCHED -> IDLE (normal flow after callback invocation) + * PENDING -> IDLE (during system deinit cleanup) + * Any state -> IDLE (this function doesn't check current state) + * + * @param entry Pointer to the CallbackEntry to reset. + * Must not be NULL. */ static void registry_reset_slot(CallbackEntry *entry) { @@ -580,22 +760,53 @@ static void registry_reset_slot(CallbackEntry *entry) * SIGNAL DATA HELPERS * ======================================================================== */ -/** - * @brief Parse GVariant into InternalSignalData - * - * Expected signature: (tiissss) - * t handler_id (uint64) - identifies which client this is for - * i result_code - * i status_code - * s current_version - * s available_version - * s update_details - * s status_message +/* + * internal_parse_signal_data - Extract fields from CheckForUpdateComplete signal. + * + * PURPOSE: + * The daemon broadcasts a D-Bus signal with a GVariant of type "(tiissss)". + * This function unpacks that GVariant into an InternalSignalData struct + * with individual typed fields, making the data easy to work with. + * + * GVariant TYPE "(tiissss)" -- what each letter means: + * '(' and ')' = tuple delimiters (the whole thing is a tuple) + * 't' = uint64 (guint64) -- handler_id + * 'i' = int32 (gint32) -- result_code (0=success, 1=fail) + * 'i' = int32 (gint32) -- status_code (0=available, 1=not, 3=error) + * 's' = string (gchar*) -- current_version (e.g., "RDKV_7.0") + * 's' = string (gchar*) -- available_version (e.g., "RDKV_8.0") + * 's' = string (gchar*) -- update_details (pipe-separated "Key:Value|...") + * 's' = string (gchar*) -- status_message (human-readable text) + * + * WHY strdup() EACH STRING? + * g_variant_get() with 's' type returns pointers into the GVariant's + * internal buffer. Those pointers are only valid while the GVariant + * exists. After on_check_complete_signal() returns, GLib may free + * the GVariant. We strdup() to create our own heap copies that survive + * beyond the GVariant's lifetime. + * + * The strdup'd copies are freed later by internal_cleanup_signal_data(). + * + * VALIDATION: + * We check the GVariant type signature before extracting. If the daemon + * sends a signal with a different signature (protocol mismatch, daemon + * version skew), we reject it immediately rather than crashing on + * mismatched g_variant_get(). + * + * THREAD: Called on the BG thread (from on_check_complete_signal). + * + * @param parameters The GVariant payload from the D-Bus signal. + * Type must be "(tiissss)". + * @param out_data Output struct. Must be zero-initialized by caller. + * On success, contains result_code, status_code, and + * 4 strdup'd strings (any may be NULL if daemon sent NULL). + * @return true on success, false if parameters is NULL or wrong type. */ bool internal_parse_signal_data(GVariant *parameters, InternalSignalData *out_data) { if (parameters == NULL || out_data == NULL) return false; + /* Verify type signature before extracting */ const gchar *sig = g_variant_get_type_string(parameters); if (strcmp(sig, "(tiissss)") != 0) { FWUPMGR_ERROR("internal_parse_signal_data: unexpected signature '%s'\n", sig); @@ -609,6 +820,7 @@ bool internal_parse_signal_data(GVariant *parameters, InternalSignalData *out_da g_variant_get(parameters, "(tiissss)", &handler_id, &result, &status, &cur, &avail, &details, &msg); + /* strdup strings to outlive the GVariant; freed by internal_cleanup_signal_data() */ out_data->result_code = (int32_t)result; out_data->status_code = (int32_t)status; out_data->current_version = cur ? strdup(cur) : NULL; @@ -619,6 +831,26 @@ bool internal_parse_signal_data(GVariant *parameters, InternalSignalData *out_da return true; } +/* + * internal_cleanup_signal_data - Free the strdup'd strings in InternalSignalData. + * + * PURPOSE: + * Called after dispatch_all_pending() has finished invoking all callbacks. + * Frees the 4 heap-allocated strings that internal_parse_signal_data() + * created via strdup(). Also zeroes the struct as a defensive measure. + * + * WHY memset AFTER free()? + * After freeing the pointers, the struct still contains the old pointer + * values (dangling pointers). If someone accidentally reads the struct + * after cleanup, they'd get use-after-free. memset to zero sets all + * pointers to NULL (safe to dereference for a NULL check) and all + * integers to 0. + * + * free(NULL) is safe in C -- it's a no-op. So if any string was NULL + * (daemon didn't send it), the free() call is harmless. + * + * @param data The InternalSignalData to clean up. Must not be NULL. + */ void internal_cleanup_signal_data(InternalSignalData *data) { free(data->current_version); @@ -628,6 +860,32 @@ void internal_cleanup_signal_data(InternalSignalData *data) memset(data, 0, sizeof(InternalSignalData)); } +/* + * internal_map_status_code - Convert daemon's integer to our public enum. + * + * PURPOSE: + * The daemon sends status_code as a plain integer in the D-Bus signal. + * Our public API uses a typed enum (CheckForUpdateStatus). This function + * does the mapping. + * + * MAPPING: + * 0 -> FIRMWARE_AVAILABLE (new firmware exists, UpdateDetails populated) + * 1 -> FIRMWARE_NOT_AVAILABLE (device is on latest version) + * 2 -> UPDATE_NOT_ALLOWED (device policy prevents updates) + * 3 -> FIRMWARE_CHECK_ERROR (XConf query failed, network error, etc.) + * 4 -> IGNORE_OPTOUT (update available, ignore opt-out preference) + * 5 -> BYPASS_OPTOUT (update available, bypass opt-out preference) + * anything else -> FIRMWARE_CHECK_ERROR (unknown code = error) + * + * WHY DEFAULT TO FIRMWARE_CHECK_ERROR? + * Unknown status codes indicate a protocol mismatch (daemon version + * newer than library). Treating unknown as "error" is the safest + * default -- the caller will handle it as a failure case rather than + * proceeding with potentially incorrect firmware data. + * + * @param status_code Integer from the daemon's signal payload. + * @return Corresponding CheckForUpdateStatus enum value. + */ CheckForUpdateStatus internal_map_status_code(int32_t status_code) { switch (status_code) { @@ -638,7 +896,7 @@ CheckForUpdateStatus internal_map_status_code(int32_t status_code) case 4: return IGNORE_OPTOUT; case 5: return BYPASS_OPTOUT; default: - FWUPMGR_ERROR("internal_map_status_code: unknown %d → FIRMWARE_CHECK_ERROR\n", + FWUPMGR_ERROR("internal_map_status_code: unknown %d -> FIRMWARE_CHECK_ERROR\n", status_code); return FIRMWARE_CHECK_ERROR; } @@ -664,13 +922,41 @@ static void dwnl_registry_reset_slot(DwnlCallbackEntry *entry); /* ======================================================================== * DOWNLOAD REGISTRY CLEANUP - * - * Called from internal_system_deinit() to free download registry resources. - * Signal unsubscription is handled by the background thread. * ======================================================================== */ -/** - * @brief Cleanup download registry — called from internal_system_deinit() +/* + * internal_dwnl_system_deinit - Free all download registry resources. + * + * PURPOSE: + * Called from internal_system_deinit() during library unload (either + * via __attribute__((destructor)) or explicitly by unregisterProcess). + * Frees any strdup'd handle_key strings that are still in the registry + * (e.g., downloads that were in-progress when the app exits) and + * destroys the mutex. + * + * WHEN IS THIS CALLED? + * During orderly shutdown of the library. The BG thread has already + * been joined (stopped), so no concurrent access to g_dwnl_registry + * is possible. The mutex lock/unlock is purely defensive -- in theory + * no other thread can be using the registry at this point. + * + * WHY FREE handle_key's? + * If the app exits while a download is ACTIVE (e.g., download at 50% + * and app receives SIGTERM), the slot still holds a strdup'd handle_key + * that was never freed by dwnl_registry_reset_slot() (because the + * terminal COMPLETED/ERROR signal never arrived). We must free it here + * to avoid a memory leak reported by Valgrind/ASan. + * + * WHY pthread_mutex_destroy()? + * The mutex was initialized by pthread_mutex_init() in internal_system_init(). + * Every init must have a matching destroy for clean resource management. + * Destroying a locked mutex is undefined behavior (we unlock first). + * + * NOTE: Signal unsubscription (g_dbus_connection_signal_unsubscribe) is + * handled separately by the BG thread during its shutdown sequence, + * NOT here. This function only handles registry memory. + * + * @param none (operates on global g_dwnl_registry) */ static void internal_dwnl_system_deinit(void) { @@ -691,15 +977,66 @@ static void internal_dwnl_system_deinit(void) * DOWNLOAD SIGNAL HANDLER * ======================================================================== */ -/** - * @brief Called by GLib when DownloadProgress signal arrives +/* + * on_download_progress_signal - BG thread entry point for DownloadProgress. + * + * PURPOSE: + * This function is called by GLib's D-Bus infrastructure when the daemon + * broadcasts the "DownloadProgress" signal. It is the ENTRY POINT for + * the "response" side of the downloadFirmware() async flow. + * + * WHEN DOES THIS FIRE? + * Repeatedly, starting shortly after downloadFirmware() was called. + * The daemon emits a DownloadProgress signal each time it has a + * progress update (0%, 10%, 50%, 100%, or on error). Unlike + * checkForUpdate which fires ONCE, this fires MANY TIMES. + * + * WHICH THREAD RUNS THIS? + * The BACKGROUND THREAD. Same thread that handles CheckForUpdateComplete + * and UpdateProgress. All three signals are dispatched on the same + * single BG thread. Only one signal handler runs at a time because + * they all share the same GMainContext. + * + * WHY (void) CASTS? + * GLib's signal handler signature requires 7 parameters. We only need + * 'parameters' (the GVariant payload). The (void) casts suppress + * "unused parameter" compiler warnings for the other 6. * - * Runs in the background thread — same thread as on_check_complete_signal(). + * SIGNAL PAYLOAD FORMAT: + * GVariant type "(tsuss)": + * t handler_id (uint64 -- which registered client) + * s firmware_name (string -- filename being downloaded) + * u progress_percent (uint32 -- 0 to 100) + * s status_string (string -- "NOTSTARTED"/"INPROGRESS"/"COMPLETED"/"ERROR") + * s message (string -- human-readable status message) + * + * IMPORTANT DIFFERENCE FROM checkForUpdate: + * checkForUpdate used integer status codes mapped by internal_map_status_code(). + * download uses STRING status values mapped by map_dwnl_status_string(). + * The daemon sends "INPROGRESS" not 0, "COMPLETED" not 1. + * + * MEMORY MANAGEMENT (DIFFERENT FROM checkForUpdate): + * checkForUpdate: internal_parse_signal_data() uses strdup() -> free() + * download: internal_parse_dwnl_signal_data() uses GLib's g_variant_get() + * with 's' format -> returns gchar* that caller must g_free(). + * + * The strings (firmware_name, status_string, message) are allocated by + * GLib during g_variant_get(). They stay valid until we g_free() them + * AFTER dispatch is complete. This ensures the strings are valid + * throughout all callback invocations. * * FLOW: - * 1. Parse GVariant payload → InternalDwnlSignalData - * 2. Dispatch to ALL ACTIVE download callbacks - * 3. If status is COMPLETED or ERROR → remove finished slots from registry + * 1. Parse GVariant -> InternalDwnlSignalData (extracts 5 fields) + * 2. Call dispatch_all_dwnl_active() -- fires all ACTIVE callbacks + * 3. g_free() the 3 GLib-allocated strings + * + * @param conn The BG thread's persistent D-Bus connection (:1.141) + * @param sender D-Bus sender address (ignored -- accept from any) + * @param object_path D-Bus object path of the signal source (ignored) + * @param interface_name D-Bus interface the signal belongs to (ignored) + * @param signal_name "DownloadProgress" (ignored -- we know from subscription) + * @param parameters The GVariant payload -- type "(tsuss)" + * @param user_data NULL (we use globals, no per-subscription user data) */ static void on_download_progress_signal(GDBusConnection *conn, const gchar *sender, @@ -709,11 +1046,13 @@ static void on_download_progress_signal(GDBusConnection *conn, GVariant *parameters, gpointer user_data) { + /* Suppress unused parameter warnings */ (void)conn; (void)sender; (void)object_path; (void)interface_name; (void)signal_name; (void)user_data; FWUPMGR_INFO("on_download_progress_signal: received\n"); + /* Parse signal payload "(tsuss)" into local struct */ InternalDwnlSignalData signal_data; memset(&signal_data, 0, sizeof(signal_data)); @@ -728,34 +1067,85 @@ static void on_download_progress_signal(GDBusConnection *conn, signal_data.progress_percent, signal_data.status_string ? signal_data.status_string : "(null)"); + /* Dispatch to all ACTIVE download callbacks */ dispatch_all_dwnl_active(&signal_data); - // Free allocated strings from g_variant_get + /* Free GLib-allocated strings from g_variant_get() */ g_free(signal_data.firmware_name); g_free(signal_data.status_string); g_free(signal_data.message); } -/** - * @brief Dispatch DownloadProgress signal to every ACTIVE download callback +/* + * dispatch_all_dwnl_active - Two-phase dispatch for download progress. + * + * PURPOSE: + * Called by on_download_progress_signal() on the BG thread. + * Finds ALL ACTIVE entries in g_dwnl_registry and invokes each one's + * callback with the progress and status. If the download ended + * (COMPLETED or ERROR), resets the slot to IDLE after the callback. + * + * TWO-PHASE DESIGN (SAME PATTERN AS checkForUpdate's dispatch_all_pending): * - * SAME TWO-PHASE DESIGN as CheckForUpdate dispatch: + * PHASE 1 -- SNAPSHOT (mutex held, ~microseconds): + * Lock g_dwnl_registry.mutex. + * Scan all 30 slots. For each ACTIVE entry: + * - Copy callback function pointer into stack-local snapshot + * - Copy handle string (snprintf into fixed buffer) + * - Record slot index (for IDLE reset later) + * - Record whether this is a terminal signal (is_final) + * Unlock mutex. * - * PHASE 1 (mutex held): - * Snapshot all ACTIVE entries. - * Do NOT change state yet — slot must stay ACTIVE for future signals. - * EXCEPTION: if status is COMPLETED or ERROR, mark slot for removal. - * Release mutex. + * PHASE 2 -- INVOKE (no mutex held, may take milliseconds): + * For each snapshot entry: + * - Call: callback(progress_percent, status) + * - If is_final: re-lock mutex, reset slot to IDLE, unlock * - * PHASE 2 (mutex released): - * Invoke each callback: callback(progress_per, status) - * Re-acquire mutex to reset completed/errored slots to IDLE. + * WHY TWO PHASES? + * Same deadlock prevention as checkForUpdate. If we held the mutex + * while calling the app's callback, and the callback tried to call + * downloadFirmware() or unregisterProcess(), that would try to lock + * the same mutex -> DEADLOCK. By releasing before invoking, the + * callback can safely call any library API. * - * WHY KEEP SLOTS ACTIVE ACROSS MULTIPLE SIGNALS? - * Download progress fires many times: 1%, 5%, 20%...100%. - * If we reset to IDLE after the first callback, subsequent signals - * would find no registered callback and be silently dropped. - * The slot only becomes IDLE when the download ends. + * KEY DIFFERENCE FROM checkForUpdate's dispatch_all_pending(): + * + * checkForUpdate: slot goes PENDING -> DISPATCHED -> IDLE after ONE callback. + * download: slot stays ACTIVE across MANY callbacks. Only goes IDLE when + * the status is DWNL_COMPLETED or DWNL_ERROR (is_final == true). + * + * For in-progress signals (is_final == false): + * - Phase 1: snapshot the ACTIVE slot, do NOT touch slot state. + * - Phase 2: invoke callback, do NOT reset slot. + * - Result: slot remains ACTIVE for the next DownloadProgress signal. + * + * For terminal signals (is_final == true): + * - Phase 1: same snapshot. + * - Phase 2: invoke callback, THEN lock mutex and reset slot to IDLE. + * - Result: slot is freed. No more callbacks will fire for this handle. + * + * STATUS MAPPING: + * The daemon sends status as a STRING ("INPROGRESS", "COMPLETED", etc.). + * map_dwnl_status_string() converts it to the DownloadStatus enum: + * "NOTSTARTED" or "INPROGRESS" -> DWNL_IN_PROGRESS + * "COMPLETED" -> DWNL_COMPLETED + * "ERROR" or "DWNL_ERROR" -> DWNL_ERROR + * + * is_final is true only for DWNL_COMPLETED or DWNL_ERROR. + * All other statuses (IN_PROGRESS) keep the slot alive. + * + * CALLBACK SIGNATURE: + * void callback(int progress_per, DownloadStatus status) + * - progress_per: 0-100 integer (percentage of download complete) + * - status: DWNL_IN_PROGRESS, DWNL_COMPLETED, or DWNL_ERROR + * - No handle parameter (different from checkForUpdate's callback) + * - Runs on BG thread, NOT the app's main thread + * + * THREAD: Runs entirely on the BG thread. + * + * @param signal_data Parsed DownloadProgress signal payload. + * Must remain valid for the duration of this function + * (strings freed by caller AFTER this returns). */ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) { @@ -763,12 +1153,13 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) DownloadCallback callback; char handle_copy[256]; int slot_index; - bool is_final; /* true if COMPLETED or ERROR — remove after firing */ + bool is_final; } DwnlSnapshot; DwnlSnapshot snapshots[MAX_PENDING_CALLBACKS]; int count = 0; + /* Map status string to enum before entering mutex */ DownloadStatus status = map_dwnl_status_string(signal_data->status_string); bool is_final = (status == DWNL_COMPLETED || status == DWNL_ERROR); @@ -779,6 +1170,7 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) DwnlCallbackEntry *e = &g_dwnl_registry.entries[i]; if (e->state != DWNL_CB_STATE_ACTIVE) continue; + /* Copy entry data into stack-local snapshot */ snapshots[count].callback = e->callback; snapshots[count].slot_index = i; snapshots[count].is_final = is_final; @@ -786,11 +1178,6 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) sizeof(snapshots[count].handle_copy), "%s", e->handle_key ? e->handle_key : ""); - /* - * If this is the final signal (completed/error), mark the slot - * so we reset it to IDLE after the callback fires. - * For in-progress signals, leave the slot ACTIVE. - */ count++; FWUPMGR_INFO("dispatch_all_dwnl_active: queued handle='%s' progress=%d%% final=%d\n", @@ -798,6 +1185,7 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) signal_data->progress_percent, is_final); } + /* Unlock before invoking callbacks (deadlock prevention) */ pthread_mutex_unlock(&g_dwnl_registry.mutex); FWUPMGR_INFO("dispatch_all_dwnl_active: %d callback(s) to fire\n", count); @@ -809,17 +1197,9 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) FWUPMGR_INFO("dispatch_all_dwnl_active: invoking callback for handle='%s'\n", s->handle_copy); - /* - * Callback signature: void fn(int progress_per, DownloadStatus status) - * No handle parameter — matches the DownloadCallback typedef exactly. - */ s->callback(signal_data->progress_percent, status); - /* - * If download is done (COMPLETED or ERROR), reset slot to IDLE. - * This frees the handle_key and makes the slot available for reuse. - * For in-progress signals, leave slot ACTIVE for next signal. - */ + /* If terminal signal, reset slot to IDLE */ if (s->is_final) { pthread_mutex_lock(&g_dwnl_registry.mutex); dwnl_registry_reset_slot(&g_dwnl_registry.entries[s->slot_index]); @@ -835,19 +1215,56 @@ static void dispatch_all_dwnl_active(const InternalDwnlSignalData *signal_data) * DOWNLOAD REGISTRY OPERATIONS * ======================================================================== */ -/** - * @brief Register a download callback keyed by handle +/* + * internal_dwnl_register_callback - Allocate a download registry slot. * - * Sets slot state to ACTIVE. Slot will receive ALL subsequent - * DownloadProgress signals until DWNL_COMPLETED or DWNL_ERROR. + * PURPOSE: + * Called by downloadFirmware() (on the main thread) to register the + * app's DownloadCallback in g_dwnl_registry. After this call, the + * BG thread will invoke the callback on EVERY DownloadProgress signal + * until the download completes or errors. * - * SAME HANDLE TWICE: - * Overwrites existing ACTIVE slot for the same handle. - * Prevents stale callbacks from a previous download session. + * REGISTRY DETAILS (g_dwnl_registry -- SEPARATE from g_registry): + * - 30 slots (DwnlCallbackEntry entries[MAX_PENDING_CALLBACKS]) + * - Protected by g_dwnl_registry.mutex (its own mutex, independent) + * - State machine: IDLE -> ACTIVE -> IDLE + * - No PENDING or DISPATCHED states (unlike checkForUpdate) + * - Slot stays ACTIVE across MULTIPLE DownloadProgress signals + * + * SLOT LIFECYCLE: + * IDLE: Slot is empty. handle_key==NULL, callback==NULL, state==0. + * ACTIVE: Slot is registered. Callback fires on every DownloadProgress. + * Stays ACTIVE until DWNL_COMPLETED or DWNL_ERROR arrives. + * IDLE: Reset by dwnl_registry_reset_slot() after terminal signal. + * + * DEDUP / OVERWRITE: + * If the same handle already has an ACTIVE entry (e.g., the app called + * downloadFirmware() twice without waiting for the first to complete), + * we OVERWRITE the existing slot. This prevents: + * - Two ACTIVE entries for the same handle (double-firing callbacks) + * - Ghost entries from a previous download that was abandoned + * We free the old handle_key before replacing it. + * + * SCAN ORDER: + * Linear scan from slot 0 to 29. We look for two things simultaneously: + * 1. existing_slot: an ACTIVE entry with the same handle (overwrite it) + * 2. free_slot: the first IDLE entry (use it if no existing found) + * If existing_slot is found, we break immediately (priority: overwrite). + * If neither is found after scanning all 30, registry is full -> fail. * - * @param handle App's FirmwareInterfaceHandle (strdup'd internally) - * @param callback App's DownloadCallback - * @return true on success, false if registry full + * THREAD SAFETY: + * Called on the MAIN thread (from downloadFirmware). + * g_dwnl_registry.mutex protects against concurrent access from: + * - Another main-thread downloadFirmware() call (unlikely, but safe) + * - The BG thread's dispatch_all_dwnl_active() reading the registry + * + * MEMORY: + * handle is strdup'd (heap copy). Freed by dwnl_registry_reset_slot() + * when the slot is released (after COMPLETED/ERROR). + * + * @param handle The handle string (e.g., "1"). Will be strdup'd. + * @param callback The app's DownloadCallback function pointer. + * @return true if registered successfully, false if registry full. */ bool internal_dwnl_register_callback(FirmwareInterfaceHandle handle, DownloadCallback callback) @@ -857,6 +1274,7 @@ bool internal_dwnl_register_callback(FirmwareInterfaceHandle handle, DwnlCallbackEntry *free_slot = NULL; DwnlCallbackEntry *existing_slot = NULL; + /* Single-pass scan: look for dedup match (same handle) and first free slot */ for (int i = 0; i < MAX_PENDING_CALLBACKS; i++) { DwnlCallbackEntry *e = &g_dwnl_registry.entries[i]; @@ -881,6 +1299,7 @@ bool internal_dwnl_register_callback(FirmwareInterfaceHandle handle, return false; } + /* If overwriting, free old handle_key to avoid leak */ if (existing_slot) { FWUPMGR_INFO("internal_dwnl_register_callback: overwriting existing for handle='%s'\n", handle); @@ -888,6 +1307,7 @@ bool internal_dwnl_register_callback(FirmwareInterfaceHandle handle, target->handle_key = NULL; } + /* Populate slot: strdup handle, store callback, mark ACTIVE */ target->handle_key = strdup(handle); target->callback = callback; target->state = DWNL_CB_STATE_ACTIVE; @@ -899,9 +1319,41 @@ bool internal_dwnl_register_callback(FirmwareInterfaceHandle handle, return true; } -/** - * @brief Reset a download registry slot to IDLE - * MUST be called with g_dwnl_registry.mutex held. +/* + * dwnl_registry_reset_slot - Clear a download registry slot back to IDLE. + * + * PURPOSE: + * Called after a terminal DownloadProgress signal (COMPLETED or ERROR) + * has been dispatched. Frees the strdup'd handle_key and resets all + * fields to zero/NULL/IDLE so the slot can be reused by a future + * downloadFirmware() call. + * + * PRECONDITION: + * MUST be called with g_dwnl_registry.mutex held by the caller. + * The caller (dispatch_all_dwnl_active Phase 2) acquires the mutex + * before calling this and releases it after. + * + * WHAT GETS FREED: + * - handle_key: strdup'd in internal_dwnl_register_callback(). + * Must be freed to avoid memory leak. Set to NULL after free. + * + * WHAT GETS ZEROED: + * - callback: set to NULL (dangling pointer prevention) + * - registered_time: set to 0 (slot has no registration timestamp) + * - state: set to DWNL_CB_STATE_IDLE (slot is free for reuse) + * + * AFTER THIS CALL: + * The slot looks exactly like it did after internal_system_init(): + * all zeros, state == IDLE, ready for a new registration. + * Subsequent DownloadProgress signals will skip this slot because + * dispatch_all_dwnl_active() only processes ACTIVE slots. + * + * free(NULL) SAFETY: + * If handle_key is already NULL (shouldn't happen, but defensive), + * free(NULL) is a safe no-op in C. + * + * @param entry Pointer to the DwnlCallbackEntry to reset. + * MUST NOT be NULL. */ static void dwnl_registry_reset_slot(DwnlCallbackEntry *entry) { @@ -918,18 +1370,55 @@ static void dwnl_registry_reset_slot(DwnlCallbackEntry *entry) * DOWNLOAD SIGNAL DATA HELPERS * ======================================================================== */ -/** - * @brief Parse GVariant DownloadProgress signal payload +/* + * internal_parse_dwnl_signal_data - Extract fields from DownloadProgress signal. + * + * PURPOSE: + * The daemon broadcasts DownloadProgress as a GVariant of type "(tsuss)". + * This function unpacks that GVariant into an InternalDwnlSignalData struct + * with individual typed fields. + * + * GVariant TYPE "(tsuss)" -- what each letter means: + * '(' and ')' = tuple delimiters + * 't' = uint64 (guint64) -- handler_id (which client this is for) + * 's' = string (gchar*) -- firmware_name (file being downloaded) + * 'u' = uint32 (guint32) -- progress_percent (0-100) + * 's' = string (gchar*) -- status_string ("INPROGRESS", "COMPLETED", etc.) + * 's' = string (gchar*) -- message (human-readable) + * + * MEMORY MODEL (DIFFERENT FROM checkForUpdate): + * checkForUpdate used strdup() on strings returned by g_variant_get('s'). + * For download, we DON'T strdup. Instead, g_variant_get() with 's' format + * returns a NEWLY-ALLOCATED gchar* that the caller must g_free(). + * + * Wait -- isn't 's' supposed to return a pointer into the GVariant? + * Actually NO: GLib documentation says for g_variant_get(): + * 's' format: returns a newly-allocated copy (gchar*) that caller frees. + * '&s' format: returns a pointer into the GVariant (no allocation). + * We use 's' (not '&s'), so we get fresh allocations that outlive the GVariant. + * + * The caller (on_download_progress_signal) calls g_free() on all three + * string pointers AFTER dispatch is complete. + * + * VALIDATION: + * Checks the GVariant type is "(tsuss)" before extracting. If the daemon + * sends a different signature (version mismatch), we reject immediately. * - * Expected GVariant signature: (ii) - * i progress_percent (0–100) - * i status_code (maps to DownloadStatus) + * THREAD: Called on the BG thread (from on_download_progress_signal). + * + * @param parameters The GVariant payload from the DownloadProgress signal. + * Type must be "(tsuss)". + * @param out_data Output struct. Must be zero-initialized by caller. + * On success, contains handler_id, progress, and 3 strings. + * Strings are GLib-allocated -- caller must g_free() them. + * @return true on success, false if parameters is NULL or wrong type. */ bool internal_parse_dwnl_signal_data(GVariant *parameters, InternalDwnlSignalData *out_data) { if (parameters == NULL || out_data == NULL) return false; + /* Verify type signature before extracting */ const gchar *sig = g_variant_get_type_string(parameters); if (strcmp(sig, "(tsuss)") != 0) { FWUPMGR_ERROR("internal_parse_dwnl_signal_data: unexpected signature '%s' (expected '(tsuss)')\n", sig); @@ -949,35 +1438,88 @@ bool internal_parse_dwnl_signal_data(GVariant *parameters, &status_str, &message_str); + /* Transfer ownership of GLib-allocated strings to caller (must g_free) */ out_data->handler_id = handler_id; - out_data->firmware_name = firmware_name; // Caller must g_free + out_data->firmware_name = firmware_name; out_data->progress_percent = progress; - out_data->status_string = status_str; // Caller must g_free - out_data->message = message_str; // Caller must g_free + out_data->status_string = status_str; + out_data->message = message_str; return true; } -/** - * @brief Map status string to DownloadStatus enum +/* + * internal_map_dwnl_status_code - Map integer status to DownloadStatus enum. + * + * PURPOSE: + * Legacy function kept for backward compatibility. In the current protocol, + * the daemon sends status as a STRING ("INPROGRESS", "COMPLETED", "ERROR") + * and the actual mapping is done by map_dwnl_status_string() below. + * + * This function exists for cases where an integer status code is received + * (older daemon versions or internal testing). + * + * MAPPING: + * 0 -> DWNL_IN_PROGRESS (download is actively happening) + * 1 -> DWNL_COMPLETED (download finished successfully) + * 2 -> DWNL_ERROR (download failed) + * anything else -> DWNL_ERROR (unknown = treat as failure) + * + * @param status_code Integer status from an older protocol format. + * @return Corresponding DownloadStatus enum value. */ DownloadStatus internal_map_dwnl_status_code(int32_t status_code) { - // This function is kept for backward compatibility but now receives - // a mapped value. The actual mapping happens in the caller. switch (status_code) { case 0: return DWNL_IN_PROGRESS; case 1: return DWNL_COMPLETED; case 2: return DWNL_ERROR; default: - FWUPMGR_ERROR("internal_map_dwnl_status_code: unknown %d → DWNL_ERROR\n", + FWUPMGR_ERROR("internal_map_dwnl_status_code: unknown %d -> DWNL_ERROR\n", status_code); return DWNL_ERROR; } } -/** - * @brief Map status string from daemon to DownloadStatus enum +/* + * map_dwnl_status_string - Map daemon's status string to DownloadStatus enum. + * + * PURPOSE: + * The daemon sends download status as a human-readable string in the + * DownloadProgress signal. This function converts that string to the + * typed DownloadStatus enum that the app's callback receives. + * + * WHY STRINGS INSTEAD OF INTEGERS? + * The daemon team chose strings for DownloadProgress (unlike + * CheckForUpdateComplete which uses integers). Strings are more + * debuggable in D-Bus tools (dbus-monitor shows "COMPLETED" not "1") + * but require strcmp-based mapping in the library. + * + * MAPPING: + * "INPROGRESS" -> DWNL_IN_PROGRESS (download actively downloading) + * "NOTSTARTED" -> DWNL_IN_PROGRESS (download queued, about to start) + * "COMPLETED" -> DWNL_COMPLETED (file fully downloaded) + * "ERROR" -> DWNL_ERROR (download failed) + * "DWNL_ERROR" -> DWNL_ERROR (alternate error string from daemon) + * NULL -> DWNL_ERROR (missing field = error) + * anything else -> DWNL_ERROR (unknown = error, with log) + * + * WHY "NOTSTARTED" MAPS TO IN_PROGRESS: + * "NOTSTARTED" is the daemon's first signal saying "I received your + * request and queued it." From the app's perspective, this is the + * beginning of the download process -- it's "in progress" even if + * bytes haven't started flowing yet. There's no separate enum for + * "queued but not started" -- the app just sees 0% IN_PROGRESS. + * + * TERMINAL vs NON-TERMINAL: + * The return value determines whether dispatch_all_dwnl_active() + * resets the slot: + * DWNL_IN_PROGRESS -> slot stays ACTIVE (more signals coming) + * DWNL_COMPLETED -> slot reset to IDLE (download ended) + * DWNL_ERROR -> slot reset to IDLE (download ended) + * + * @param status_str String from the daemon's signal. May be NULL. + * @return Corresponding DownloadStatus enum value. */ static DownloadStatus map_dwnl_status_string(const char *status_str) { @@ -993,22 +1535,74 @@ static DownloadStatus map_dwnl_status_string(const char *status_str) return DWNL_ERROR; } - FWUPMGR_ERROR("map_dwnl_status_string: unknown status '%s' → DWNL_ERROR\n", status_str); + /* Unknown string -- treat as error */ + FWUPMGR_ERROR("map_dwnl_status_string: unknown status '%s' -> DWNL_ERROR\n", status_str); return DWNL_ERROR; } /* ======================================================================== - * UPDATE FIRMWARE — INTERNAL ENGINE + * UPDATE FIRMWARE -- INTERNAL ENGINE * ======================================================================== * - * Mirror of the DownloadFirmware engine above. - * Same registry pattern, same two-phase dispatch, same lifecycle. + * This section contains all the internal machinery that powers the + * updateFirmware() public API. It is the third and final async engine + * in the library, mirroring the download engine above. + * + * ARCHITECTURE OVERVIEW: + * + * updateFirmware() [_api.c, main thread] + * | + * +--> internal_update_register_callback() [registers in g_update_registry] + * +--> g_dbus_connection_call() [fire-and-forget to daemon] + * | + * [daemon flashes firmware, broadcasts UpdateProgress signals] + * | + * on_update_progress_signal() [BG thread, GLib callback] + * | + * +--> internal_parse_update_signal_data() [extract "(tsiis)" payload] + * +--> dispatch_all_update_active() [two-phase dispatch] + * | + * +--> internal_map_update_status_code() [int -> UpdateStatus] + * +--> callback(progress, status) [app's function] + * +--> update_registry_reset_slot() [if terminal] + * + * KEY DIFFERENCES FROM DOWNLOAD ENGINE: + * + * Signal format: + * Download: "(tsuss)" -- progress is uint32, status is STRING + * Update: "(tsiis)" -- progress is int32, status is INTEGER + * + * Status mapping: + * Download: map_dwnl_status_string() uses strcmp on strings + * Update: internal_map_update_status_code() uses switch on integers + * + * Strings to free after parsing: + * Download: 3 (firmware_name, status_string, message) + * Update: 2 (firmware_name, message) -- no status_string + * + * Registry: + * Download: g_dwnl_registry with DwnlCallbackEntry and DWNL_CB_STATE_* + * Update: g_update_registry with UpdateCbEntry and UPDATE_CB_STATE_* + * + * Slot lifecycle (same as download): + * IDLE -> ACTIVE (on register) -> ACTIVE (fires repeatedly) -> IDLE (on terminal) + * + * SIGNAL: UpdateProgress "(tsiis)" + * t handler_id -- uint64, identifies the registered client + * s firmware_name -- string, image being flashed + * i progress_percent -- int32, 0 to 100 + * i status_code -- int32, 0=IN_PROGRESS, 1=COMPLETED, 2=ERROR + * s message -- string, human-readable status * - * Signal: UpdateProgress (ii) — progress_percent, status_code * Registry slot: ACTIVE until UPDATE_COMPLETED or UPDATE_ERROR, then IDLE. * ======================================================================== */ /* ---- Forward declarations for helper functions ---- */ +/* + * These forward declarations allow the functions to be defined in a + * logical order (signal handler first, then dispatch, then helpers) + * even though the C compiler needs to see declarations before use. + */ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_data); static void update_registry_reset_slot(UpdateCbEntry *entry); @@ -1017,10 +1611,43 @@ static void update_registry_reset_slot(UpdateCbEntry *entry); * ======================================================================== */ /** - * @brief Cleanup update registry — frees all strdup'd handle_key strings + * @brief Cleanup update registry -- frees all strdup'd handle_key strings + * + * PURPOSE: + * Called from internal_system_deinit() during library shutdown + * (unregisterProcess -> internal_system_deinit -> this function). + * Walks all 30 registry slots and frees any handle_key strings that + * were allocated by strdup() in internal_update_register_callback(). + * + * WHY THIS IS NEEDED: + * When the library shuts down, any ACTIVE update callbacks are + * abandoned (no more signals will be dispatched). But the strdup'd + * handle_key strings are still on the heap. Without this cleanup, + * they would leak. Valgrind would report "definitely lost" blocks. + * + * WHAT ABOUT THE CALLBACKS THEMSELVES: + * Callback function pointers are not heap-allocated -- they're just + * pointers to compiled code. Setting callback=NULL is defensive but + * doesn't free anything. The ONLY heap allocation per slot is + * handle_key (from strdup). + * + * SIGNAL UNSUBSCRIPTION: + * This function does NOT unsubscribe from the UpdateProgress D-Bus + * signal. That's handled by the BG thread's cleanup code when it + * calls g_dbus_connection_signal_unsubscribe(). The signal + * subscription and the registry are independent concerns. + * + * MUTEX DESTRUCTION: + * After freeing all strings, pthread_mutex_destroy() is called to + * release the mutex's internal resources. After this, the mutex + * must NOT be used again -- any lock/unlock would be undefined behavior. + * + * THREAD SAFETY: + * Called during shutdown when the BG thread has already been stopped. + * The mutex lock/unlock is still used for correctness, even though + * no other thread should be accessing the registry at this point. * - * Called from internal_system_deinit(). Signal unsubscription is handled - * by the background thread. + * Called from: internal_system_deinit() (in this file) */ static void internal_update_system_deinit(void) { @@ -1039,13 +1666,66 @@ static void internal_update_system_deinit(void) /* ======================================================================== * UPDATE SIGNAL HANDLER + * + * When the daemon broadcasts an "UpdateProgress" D-Bus signal, GLib's + * event loop on the BG thread dispatches it to on_update_progress_signal(). + * That function parses the signal, then calls dispatch_all_update_active() + * to invoke all registered UpdateCallbacks. * ======================================================================== */ /** - * @brief Called by GLib when UpdateProgress signal arrives + * @brief Called by GLib when UpdateProgress signal arrives on D-Bus + * + * PURPOSE: + * This is the BG thread's entry point for handling firmware update + * progress signals. When the daemon flashes firmware, it periodically + * broadcasts UpdateProgress signals on D-Bus. GLib's GMainLoop on + * the BG thread receives these signals and invokes THIS function. + * + * EXECUTION CONTEXT: + * Runs on the BACKGROUND THREAD (not the main thread). + * Called by g_main_loop_run() -> GLib signal dispatch. + * The BG thread subscribed to "UpdateProgress" signals during + * internal_system_init() using g_dbus_connection_signal_subscribe(). + * + * SIGNAL FORMAT -- "(tsiis)": + * t handler_id -- uint64, identifies which registered client + * s firmware_name -- string, the image being flashed + * i progress_percent -- int32, 0 to 100 + * i status_code -- int32, 0=IN_PROGRESS, 1=COMPLETED, 2=ERROR + * s message -- string, human-readable status message + * + * NOTE: This is DIFFERENT from DownloadProgress's "(tsuss)": + * - Download uses uint32 for progress, update uses int32 + * - Download uses string for status ("INPROGRESS"), update uses int32 + * - Download has 3 g_free-able strings, update has 2 + * + * FLOW: + * 1. Suppress unused parameter warnings with (void) casts + * 2. Zero-initialize InternalUpdateSignalData on the stack + * 3. Call internal_parse_update_signal_data() to extract fields + * 4. Log the parsed data for debugging + * 5. Call dispatch_all_update_active() to invoke all ACTIVE callbacks + * 6. Free heap-allocated strings (firmware_name, message) from g_variant_get + * + * MEMORY OWNERSHIP: + * g_variant_get() with "s" format allocates new strings on the heap + * via g_strdup(). The caller (this function) MUST g_free() them. + * Two strings need freeing: firmware_name and message. + * (Compare: download has three -- firmware_name, status_string, message) * - * Runs in background thread. Parses payload and dispatches to all - * ACTIVE update callbacks. + * THREAD SAFETY: + * This function itself is single-threaded (only the BG thread calls it). + * But it calls dispatch_all_update_active() which accesses the shared + * g_update_registry under mutex protection. + * + * @param conn The BG thread's persistent D-Bus connection + * @param sender The D-Bus sender (daemon's unique name) + * @param object_path D-Bus object path ("/org/rdkfwupdater/Service") + * @param interface_name D-Bus interface ("org.rdkfwupdater.Interface") + * @param signal_name "UpdateProgress" + * @param parameters GVariant containing the "(tsiis)" payload + * @param user_data NULL (not used) */ static void on_update_progress_signal(GDBusConnection *conn, const gchar *sender, @@ -1055,11 +1735,13 @@ static void on_update_progress_signal(GDBusConnection *conn, GVariant *parameters, gpointer user_data) { + /* Suppress unused parameter warnings */ (void)conn; (void)sender; (void)object_path; (void)interface_name; (void)signal_name; (void)user_data; FWUPMGR_INFO("on_update_progress_signal: received\n"); + /* Parse signal payload "(tsiis)" into local struct */ InternalUpdateSignalData signal_data; memset(&signal_data, 0, sizeof(signal_data)); @@ -1074,9 +1756,10 @@ static void on_update_progress_signal(GDBusConnection *conn, signal_data.progress_percent, signal_data.status_code); + /* Dispatch to all ACTIVE update callbacks */ dispatch_all_update_active(&signal_data); - // Free allocated strings from g_variant_get + /* Free GLib-allocated strings from g_variant_get() */ g_free(signal_data.firmware_name); g_free(signal_data.message); } @@ -1084,17 +1767,59 @@ static void on_update_progress_signal(GDBusConnection *conn, /** * @brief Dispatch UpdateProgress signal to every ACTIVE update callback * - * TWO-PHASE DESIGN (identical to download dispatch): + * PURPOSE: + * Called by on_update_progress_signal() after parsing the D-Bus signal + * payload. Finds ALL ACTIVE entries in g_update_registry and invokes + * their callbacks with the current progress and status. + * + * TWO-PHASE DESIGN (identical pattern to download dispatch): + * + * PHASE 1 (mutex HELD): + * - Lock g_update_registry.mutex + * - Scan all 30 slots for ACTIVE entries + * - For each ACTIVE entry, copy callback pointer, handle, and slot + * index into a local snapshot array on the stack + * - Determine if this is a terminal signal (COMPLETED or ERROR) + * - Unlock mutex + * + * PHASE 2 (mutex RELEASED): + * - Iterate through snapshot array + * - Invoke each callback(progress_percent, status) + * - If terminal signal: re-lock mutex, reset slot to IDLE, unlock + * - If in-progress: leave slot ACTIVE for the next signal + * + * WHY TWO PHASES (not one): + * If we held the mutex while invoking callbacks, the callbacks could + * not safely call any library function that touches the registry + * (e.g., updateFirmware() again, or unregisterProcess()). That would + * deadlock because our thread already holds the mutex. By releasing + * the mutex before invoking callbacks, we avoid this entirely. + * + * WHY SNAPSHOT (not direct access): + * Once we release the mutex, another thread could modify the registry + * (e.g., the main thread calling updateFirmware() to register a new + * callback). The snapshot freezes the state at scan time, so our + * iteration is safe regardless of concurrent modifications. + * + * TERMINAL vs IN-PROGRESS SIGNALS: + * - status == UPDATE_COMPLETED or UPDATE_ERROR -> TERMINAL + * The update is done (success or failure). Reset slot to IDLE so + * it can be reused for future updateFirmware() calls. + * - status == UPDATE_IN_PROGRESS -> IN-PROGRESS + * The update is still running. Leave slot ACTIVE so the NEXT + * UpdateProgress signal also dispatches to this callback. + * + * This is the key difference from checkForUpdate's dispatch: + * checkForUpdate: slot fires ONCE then goes to IDLE + * downloadFirmware: slot fires MANY times, IDLE on terminal + * updateFirmware: slot fires MANY times, IDLE on terminal (same) * - * PHASE 1 (mutex held): - * Snapshot all ACTIVE entries. - * Mark is_final=true only if status is COMPLETED or ERROR. - * Release mutex. + * THREAD SAFETY: + * Called on the BG thread. Accesses g_update_registry under mutex. + * Phase 2 callbacks run WITHOUT mutex -- the app's callback function + * can safely call library APIs without deadlocking. * - * PHASE 2 (mutex released): - * Invoke callback(progress_per, status) for each snapshot. - * If is_final: re-acquire mutex, reset slot to IDLE. - * If in-progress: leave slot ACTIVE for next signal. + * @param signal_data Parsed signal data from on_update_progress_signal() */ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_data) { @@ -1108,6 +1833,7 @@ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_da UpdateSnapshot snapshots[MAX_PENDING_CALLBACKS]; int count = 0; + /* Map status code to enum before entering mutex */ UpdateStatus status = internal_map_update_status_code(signal_data->status_code); bool is_final = (status == UPDATE_COMPLETED || status == UPDATE_ERROR); @@ -1118,6 +1844,7 @@ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_da UpdateCbEntry *e = &g_update_registry.entries[i]; if (e->state != UPDATE_CB_STATE_ACTIVE) continue; + /* Copy entry data into stack-local snapshot */ snapshots[count].callback = e->callback; snapshots[count].slot_index = i; snapshots[count].is_final = is_final; @@ -1133,6 +1860,7 @@ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_da signal_data->progress_percent, is_final); } + /* Unlock before invoking callbacks (deadlock prevention) */ pthread_mutex_unlock(&g_update_registry.mutex); FWUPMGR_INFO("dispatch_all_update_active: %d callback(s) to fire\n", count); @@ -1144,22 +1872,15 @@ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_da FWUPMGR_INFO("dispatch_all_update_active: invoking callback " "for handle='%s'\n", s->handle_copy); - /* - * Callback signature: void fn(int progress_per, UpdateStatus status) - * Matches UpdateCallback typedef exactly. - */ s->callback(signal_data->progress_percent, status); - /* - * If this was the final signal (COMPLETED or ERROR), reset slot to IDLE. - * For in-progress signals, leave slot ACTIVE for the next signal. - */ + /* If terminal signal, reset slot to IDLE */ if (s->is_final) { pthread_mutex_lock(&g_update_registry.mutex); update_registry_reset_slot(&g_update_registry.entries[s->slot_index]); pthread_mutex_unlock(&g_update_registry.mutex); - FWUPMGR_INFO("dispatch_all_update_active: slot %d → IDLE " + FWUPMGR_INFO("dispatch_all_update_active: slot %d -> IDLE " "(update ended)\n", s->slot_index); } } @@ -1167,16 +1888,71 @@ static void dispatch_all_update_active(const InternalUpdateSignalData *signal_da /* ======================================================================== * UPDATE REGISTRY OPERATIONS + * + * These functions manage the g_update_registry -- allocating slots for + * new update callbacks (internal_update_register_callback) and cleaning + * up slots when updates complete (update_registry_reset_slot). + * + * The registry holds up to MAX_PENDING_CALLBACKS (30) entries. + * Each entry has two states: UPDATE_CB_STATE_IDLE (available) and + * UPDATE_CB_STATE_ACTIVE (callback registered, waiting for signals). * ======================================================================== */ /** * @brief Register an update callback keyed by handle * - * Sets slot to ACTIVE. Slot receives ALL subsequent UpdateProgress signals - * until UPDATE_COMPLETED or UPDATE_ERROR resets it to IDLE. + * PURPOSE: + * Called by updateFirmware() (in _api.c) AFTER the D-Bus connection + * succeeds but BEFORE the fire-and-forget D-Bus call is sent. + * Allocates a slot in g_update_registry so the BG thread can find + * the callback when UpdateProgress signals arrive. + * + * HOW IT WORKS: + * 1. Lock g_update_registry.mutex (prevents races with BG thread) + * 2. Scan all MAX_PENDING_CALLBACKS (30) slots looking for: + * a. An existing ACTIVE slot with the same handle (dedup case) + * b. The first IDLE slot (normal allocation case) + * 3. Pick the target: + * - If same handle found: overwrite it (existing_slot) + * - Else if free slot found: use it (free_slot) + * - Else: return false (registry full) + * 4. Populate the target slot: + * - handle_key = strdup(handle) -- heap copy of "1" + * - callback = the UpdateCallback function pointer + * - state = UPDATE_CB_STATE_ACTIVE -- ready for dispatch + * - registered_time = time(NULL) -- unix timestamp + * 5. Unlock mutex and return true * * SAME HANDLE TWICE: - * Overwrites existing ACTIVE slot for the same handle. + * If the same handle already has an ACTIVE slot (e.g., the app calls + * updateFirmware() again before the first update finishes), the old + * entry is OVERWRITTEN. The old handle_key string is freed first to + * avoid a memory leak. This means: + * - Only ONE active update callback per handle at a time + * - The NEW callback replaces the old one + * - The old callback will never fire again + * + * WHY ACTIVE (NOT PENDING): + * checkForUpdate uses PENDING -> DISPATCHED -> IDLE (fires once). + * updateFirmware uses ACTIVE -> IDLE (fires many times until terminal). + * The slot stays ACTIVE and the callback fires on EVERY UpdateProgress + * signal until the status is UPDATE_COMPLETED or UPDATE_ERROR, at + * which point dispatch_all_update_active() resets it to IDLE. + * + * THREAD SAFETY: + * Thread-safe. Protected by g_update_registry.mutex. + * Called from the main thread (inside updateFirmware()). + * The BG thread reads the same registry in dispatch_all_update_active(). + * The mutex ensures they never read/write the same slot simultaneously. + * + * MEMORY: + * handle_key = strdup(handle) -- heap allocated by THIS function. + * Freed by update_registry_reset_slot() when the slot returns to IDLE, + * or freed here if overwriting an existing entry. + * + * @param handle The handle string from registerProcess(), e.g. "1" + * @param callback The UpdateCallback function pointer to invoke later + * @return true if registered, false if registry full (all 30 slots occupied) */ bool internal_update_register_callback(FirmwareInterfaceHandle handle, UpdateCallback callback) @@ -1186,6 +1962,7 @@ bool internal_update_register_callback(FirmwareInterfaceHandle handle, UpdateCbEntry *free_slot = NULL; UpdateCbEntry *existing_slot = NULL; + /* Single-pass scan: look for dedup match (same handle) and first free slot */ for (int i = 0; i < MAX_PENDING_CALLBACKS; i++) { UpdateCbEntry *e = &g_update_registry.entries[i]; @@ -1210,6 +1987,7 @@ bool internal_update_register_callback(FirmwareInterfaceHandle handle, return false; } + /* If overwriting, free old handle_key to avoid leak */ if (existing_slot) { FWUPMGR_INFO("internal_update_register_callback: " "overwriting existing for handle='%s'\n", handle); @@ -1217,6 +1995,7 @@ bool internal_update_register_callback(FirmwareInterfaceHandle handle, target->handle_key = NULL; } + /* Populate slot: strdup handle, store callback, mark ACTIVE */ target->handle_key = strdup(handle); target->callback = callback; target->state = UPDATE_CB_STATE_ACTIVE; @@ -1231,7 +2010,42 @@ bool internal_update_register_callback(FirmwareInterfaceHandle handle, /** * @brief Reset an update registry slot to IDLE - * MUST be called with g_update_registry.mutex held. + * + * PURPOSE: + * Returns a single UpdateCbEntry to the IDLE (empty) state so it + * can be reused by a future updateFirmware() call. Called in two + * situations: + * 1. dispatch_all_update_active() -- when a terminal signal arrives + * (UPDATE_COMPLETED or UPDATE_ERROR), the slot is reset after + * the callback is invoked. + * 2. internal_update_system_deinit() -- during library shutdown, + * all slots are cleaned up (though deinit frees handle_key + * directly rather than calling this function). + * + * WHAT IT DOES: + * 1. Frees the handle_key string (heap-allocated by strdup in + * internal_update_register_callback). Sets pointer to NULL. + * 2. Clears the callback function pointer to NULL. + * 3. Resets registered_time to 0. + * 4. Sets state back to UPDATE_CB_STATE_IDLE. + * + * After this call, the slot is indistinguishable from a never-used + * slot. It will be found by the next registry scan looking for a + * free slot. + * + * THREAD SAFETY: + * MUST be called with g_update_registry.mutex HELD by the caller. + * This function does NOT lock the mutex itself -- the caller is + * responsible for locking. This is because the caller typically + * needs to do the lock, call this function, then do other work + * before unlocking (or is already inside a locked section). + * + * MEMORY: + * Frees one heap allocation: handle_key (from strdup). + * Does NOT free the entry itself -- entries are array elements + * inside g_update_registry, not individually heap-allocated. + * + * @param entry Pointer to the UpdateCbEntry to reset. Must not be NULL. */ static void update_registry_reset_slot(UpdateCbEntry *entry) { @@ -1246,20 +2060,70 @@ static void update_registry_reset_slot(UpdateCbEntry *entry) /* ======================================================================== * UPDATE SIGNAL DATA HELPERS + * + * These helper functions handle the translation between D-Bus wire + * format and the library's internal types: + * + * internal_parse_update_signal_data() -- GVariant "(tsiis)" -> struct + * internal_map_update_status_code() -- int (0/1/2) -> UpdateStatus enum + * + * Both are pure functions with no side effects (except logging on error). * ======================================================================== */ /** - * @brief Parse GVariant UpdateProgress payload + * @brief Parse GVariant UpdateProgress payload into a struct + * + * PURPOSE: + * Extracts the five fields from the D-Bus UpdateProgress signal's + * GVariant payload and stores them in an InternalUpdateSignalData + * struct for easy access by the dispatch logic. + * + * EXPECTED GVariant SIGNATURE: "(tsiis)" + * t handler_id -- uint64: identifies which registered client + * s firmware_name -- string: the image being flashed (e.g., "firmware_v8.bin") + * i progress_percent -- int32: 0 to 100 + * i status_code -- int32: 0=IN_PROGRESS, 1=COMPLETED, 2=ERROR + * s message -- string: human-readable status message + * + * COMPARISON WITH DOWNLOAD SIGNAL: + * Download signal "(tsuss)": + * t handler_id, s firmware_name, u progress (uint32), + * s status_string ("INPROGRESS"/"COMPLETED"/"ERROR"), s message + * Update signal "(tsiis)": + * t handler_id, s firmware_name, i progress (int32), + * i status_code (0/1/2), s message + * + * Key differences: + * - Download: progress is uint32 (u), status is string (s) + * - Update: progress is int32 (i), status is int32 (i) + * - Download: 3 strings to g_free (firmware_name, status_string, message) + * - Update: 2 strings to g_free (firmware_name, message) * - * Expected GVariant signature: (ii) - * i progress_percent (0–100) - * i status_code (maps to UpdateStatus) + * MEMORY OWNERSHIP: + * g_variant_get() with "s" format ALLOCATES new strings on the heap + * (via g_strdup). The CALLER is responsible for freeing them with + * g_free() when done. This function sets: + * out_data->firmware_name -- caller must g_free() + * out_data->message -- caller must g_free() + * Integer fields (handler_id, progress_percent, status_code) are + * simple value copies -- no heap allocation. + * + * THREAD SAFETY: + * Safe -- operates only on its parameters (no global state). + * Called from on_update_progress_signal() on the BG thread. + * + * @param parameters The GVariant from the D-Bus signal. Must not be NULL. + * @param out_data Output struct to populate. Must not be NULL. + * Caller must g_free firmware_name and message. + * @return true on success, false if parameters is NULL, out_data is NULL, + * or the GVariant signature doesn't match "(tsiis)". */ bool internal_parse_update_signal_data(GVariant *parameters, InternalUpdateSignalData *out_data) { if (parameters == NULL || out_data == NULL) return false; + /* Verify type signature before extracting */ const gchar *sig = g_variant_get_type_string(parameters); if (strcmp(sig, "(tsiis)") != 0) { FWUPMGR_ERROR("internal_parse_update_signal_data: " @@ -1280,17 +2144,56 @@ bool internal_parse_update_signal_data(GVariant *parameters, &status, &message_str); + /* Transfer ownership of GLib-allocated strings to caller (must g_free) */ out_data->handler_id = handler_id; - out_data->firmware_name = firmware_name; // Caller must g_free + out_data->firmware_name = firmware_name; out_data->progress_percent = progress; out_data->status_code = status; - out_data->message = message_str; // Caller must g_free + out_data->message = message_str; return true; } /** - * @brief Map raw integer to UpdateStatus enum + * @brief Map raw integer status code to UpdateStatus enum + * + * PURPOSE: + * The daemon's UpdateProgress signal sends status as a raw integer + * (0, 1, or 2). The library's public API uses the UpdateStatus enum + * (UPDATE_IN_PROGRESS, UPDATE_COMPLETED, UPDATE_ERROR). This function + * translates between the two representations. + * + * COMPARISON WITH DOWNLOAD STATUS MAPPING: + * Download uses STRING codes: "INPROGRESS", "COMPLETED", "ERROR" + * -> mapped by map_dwnl_status_string() using strcmp() + * Update uses INTEGER codes: 0, 1, 2 + * -> mapped by THIS function using a switch statement + * + * The integer approach is simpler and faster (no string comparison), + * but less self-documenting in D-Bus traces. The two APIs evolved + * independently, which is why they use different conventions. + * + * MAPPING: + * 0 -> UPDATE_IN_PROGRESS (flashing is underway, more signals coming) + * 1 -> UPDATE_COMPLETED (flashing finished successfully) + * 2 -> UPDATE_ERROR (flashing failed) + * anything else -> UPDATE_ERROR (defensive default, with error log) + * + * WHY DEFAULT TO ERROR: + * If the daemon sends an unknown status code (e.g., 3), we treat it + * as an error. This is the SAFEST default because: + * - It causes the slot to be reset to IDLE (terminal status) + * - It notifies the app that something unexpected happened + * - It prevents the slot from staying ACTIVE forever + * If we defaulted to IN_PROGRESS, an unknown code would leave the + * slot ACTIVE indefinitely, leaking a registry slot. + * + * THREAD SAFETY: + * Pure function -- no side effects, no global state. Safe to call + * from any thread. + * + * @param status_code The raw integer from the D-Bus signal (0, 1, or 2) + * @return The corresponding UpdateStatus enum value */ UpdateStatus internal_map_update_status_code(int32_t status_code) { @@ -1300,7 +2203,7 @@ UpdateStatus internal_map_update_status_code(int32_t status_code) case 2: return UPDATE_ERROR; default: FWUPMGR_ERROR("internal_map_update_status_code: " - "unknown %d → UPDATE_ERROR\n", status_code); + "unknown %d -> UPDATE_ERROR\n", status_code); return UPDATE_ERROR; } } @@ -1310,33 +2213,88 @@ UpdateStatus internal_map_update_status_code(int32_t status_code) * ======================================================================== */ /** - * @brief Parse update_details string into UpdateDetails structure + * parse_update_details - Parse pipe-separated firmware details into a struct. + * + * OVERVIEW + * + * PURPOSE: + * When the daemon reports FIRMWARE_AVAILABLE, it includes a string + * describing the available firmware. This string uses a custom + * pipe-separated Key:Value format: + * + * "File:firmware_v8.bin|Location:http://cdn.example.com/fw|Version:RDKV_8.0|Reboot:false|Delay:false|PDRI:N/A|Peripherals:N/A" + * + * This function tokenizes that string and copies each value into the + * appropriate field of an UpdateDetails struct, which the callback + * receives via FwInfoData->UpdateDetails. + * + * WHY PIPE-SEPARATED (NOT JSON)? + * This is a daemon-internal format, not a public protocol. It's simple, + * requires no JSON parser dependency, and is easy to tokenize with + * strtok_r(). The library translates this format into typed struct + * fields so callers never see the pipe-separated format. + * + * STRING FORMAT: + * - Tokens separated by '|' (pipe) + * - Each token is "Key:Value" (colon-separated) + * - Known keys: File, Location, IPv6Location, Version, Reboot, + * Delay, PDRI, Peripherals, Protocol, CertBundle + * - "N/A" is treated as "not available" for PDRI and Peripherals + * - Unknown keys are logged and skipped (forward compatibility) + * + * THREAD SAFETY: + * Safe -- operates only on local data. The work_str is a strdup'd + * copy (so strtok_r doesn't modify the original), and out_details + * is caller-provided (stack-allocated in dispatch_all_pending). * - * The update_details string from the daemon is a comma-separated key:value format: - * "FwFileName:filename.bin,FwUrl:https://...,FwVersion:1.0,..." + * MEMORY: + * work_str is strdup'd at the start and freed at the end. + * out_details fields are char arrays (not pointers) -- data is + * copied directly into the struct, no additional heap allocation. * - * This function safely parses it and populates the UpdateDetails structure. + * ROBUSTNESS: + * - NULL/empty input is valid (returns success with zeroed struct) + * - Malformed tokens (no colon) are logged and skipped + * - Unknown keys are logged and skipped + * - strdup failure returns false (out of memory) * - * @param update_details_str Comma-separated string from daemon (may be NULL) - * @param out_details Output UpdateDetails structure (must be allocated) - * @return true if parsing succeeded (even if string was NULL/empty), - * false only on critical errors + * @param update_details_str The pipe-separated string from the daemon. + * May be NULL or empty (both are valid). + * @param out_details Output struct. Filled with parsed values. + * Caller must provide allocated storage. + * @return true on success (even if input was NULL/empty -- struct is zeroed), + * false only on critical errors (NULL out_details, OOM). * - * Thread safety: Safe - operates on local data only - * Memory: out_details is caller-allocated, this function fills arrays + * Called by: dispatch_all_pending() (Phase 2, when status == FIRMWARE_AVAILABLE) */ static bool parse_update_details(const char *update_details_str, UpdateDetails *out_details) { + /* + * NULL check on the output struct. This is a programming error + * in the caller -- should never happen, but catch it defensively. + */ if (out_details == NULL) { FWUPMGR_ERROR("parse_update_details: out_details is NULL\n"); return false; } - /* Zero-initialize the output structure */ + /* + * Zero-initialize the output struct. + * + * All char arrays (FwFileName, FwUrl, etc.) start as empty strings + * (first byte '\0'). This ensures that if a key is missing from + * the daemon's string, the corresponding field is empty rather + * than containing garbage. + */ memset(out_details, 0, sizeof(UpdateDetails)); - /* Empty or NULL input is valid - just means no details available */ + /* + * NULL or empty input is valid -- it means the daemon has no + * details to share. Return success with a zeroed struct. + * The caller (dispatch_all_pending) will see empty strings in + * all fields and can handle accordingly. + */ if (update_details_str == NULL || update_details_str[0] == '\0') { FWUPMGR_INFO("parse_update_details: empty input, returning zeroed structure\n"); return true; @@ -1344,41 +2302,129 @@ static bool parse_update_details(const char *update_details_str, FWUPMGR_INFO("parse_update_details: parsing '%s'\n", update_details_str); - /* Make a working copy since strtok modifies the string */ + /* + * Create a working copy of the input string. + * + * Why? strtok_r() MODIFIES the string it tokenizes (it replaces + * delimiters with '\0'). The input string belongs to InternalSignalData + * (from strdup in internal_parse_signal_data). We must not modify it + * because internal_cleanup_signal_data() needs to free() the original + * pointer. Modifying the string would corrupt the pointer if strtok_r + * happened to insert '\0' at a different position. + * + * strdup() allocates strlen(update_details_str)+1 bytes on the heap. + * Freed at the end of this function. + */ char *work_str = strdup(update_details_str); if (work_str == NULL) { FWUPMGR_ERROR("parse_update_details: strdup failed\n"); return false; } - /* Parse pipe-separated key:value pairs (daemon uses | not ,) */ + /* + * Tokenize the pipe-separated string. + * + * strtok_r() is the REENTRANT version of strtok(). We use it + * instead of strtok() because: + * - strtok() uses a static internal buffer -- NOT thread-safe. + * If another thread called strtok() simultaneously, they'd + * corrupt each other's state. + * - strtok_r() uses the caller-provided 'saveptr' for state, + * making it thread-safe. + * + * First call: strtok_r(work_str, "|", &saveptr) + * Returns pointer to first token (everything before first '|') + * Replaces the '|' with '\0' in work_str + * Stores position in saveptr for next call + * + * Subsequent calls: strtok_r(NULL, "|", &saveptr) + * Returns pointer to next token + * NULL when no more tokens + * + * Example: + * Input: "File:fw.bin|Version:8.0|Reboot:false" + * Call 1: returns "File:fw.bin" + * Call 2: returns "Version:8.0" + * Call 3: returns "Reboot:false" + * Call 4: returns NULL (done) + */ char *saveptr = NULL; char *token = strtok_r(work_str, "|", &saveptr); while (token != NULL) { - /* Split on ':' to get key and value */ + /* + * Each token should be "Key:Value". Find the colon separator. + * + * strchr() returns a pointer to the first ':' in the token, + * or NULL if there is no colon (malformed token). + */ char *colon = strchr(token, ':'); if (colon == NULL) { - /* Malformed token, skip it */ + /* + * No colon found -- this token is malformed. Skip it. + * This is defensive: if the daemon sends garbage like + * "File:fw.bin|OOPS|Version:8.0", we skip "OOPS" and + * continue parsing the rest. + */ FWUPMGR_ERROR("parse_update_details: malformed token '%s' (no colon)\n", token); token = strtok_r(NULL, "|", &saveptr); continue; } - /* Null-terminate the key and get the value */ + /* + * Split the token into key and value by replacing ':' with '\0'. + * + * Before: token = "File:fw.bin" (colon points to ':') + * After: key = "File" (token, now null-terminated at colon) + * value = "fw.bin" (colon + 1, rest of original string) + * + * This is an in-place split -- we're modifying our work_str copy. + */ *colon = '\0'; const char *key = token; const char *value = colon + 1; - /* Match keys and copy values into appropriate fields - * Daemon uses: File, Location, Version, Reboot, Delay, PDRI, Peripherals - * We map them to our struct fields */ + /* + * Match the key to our struct fields and copy the value. + * + * strncpy with sizeof(field)-1 ensures we never overflow the + * destination buffer. The struct fields are fixed-size arrays + * (e.g., FwFileName[128], FwUrl[512]). The -1 leaves room for + * the null terminator. + * + * We don't need to explicitly null-terminate because memset + * zeroed the entire struct at the start (all bytes are '\0'). + * strncpy will write the value characters and NOT overwrite + * the trailing '\0' that's already there from memset, as long + * as the value is shorter than the buffer. + * + * KEY MAPPING: + * Daemon key -> Struct field + * "File" -> FwFileName + * "Location" -> FwUrl (IPv4 download URL) + * "IPv6Location" -> FwUrl (IPv6 fallback, used if Location is "N/A") + * "Version" -> FwVersion + * "Reboot" -> RebootImmediately ("true" or "false") + * "Delay" -> DelayDownload ("true" or "false") + * "PDRI" -> PDRIVersion (PDRI image version) + * "Peripherals" -> PeripheralFirmwares (peripheral versions) + * "Protocol" -> (no struct field -- skipped) + * "CertBundle" -> (no struct field -- skipped) + */ if (strcmp(key, "File") == 0) { strncpy(out_details->FwFileName, value, sizeof(out_details->FwFileName) - 1); } else if (strcmp(key, "Location") == 0 || strcmp(key, "IPv6Location") == 0) { - /* Use Location if not empty, fallback to IPv6Location */ + /* + * Use Location if it's a real URL (not "N/A" and not empty). + * IPv6Location is a fallback -- if Location was "N/A" but + * IPv6Location has a URL, we use that instead. + * + * We don't overwrite an already-set FwUrl. If Location came + * first and was valid, IPv6Location won't overwrite it. + * This depends on daemon field ordering (Location before IPv6Location). + */ if (strcmp(value, "N/A") != 0 && value[0] != '\0') { strncpy(out_details->FwUrl, value, sizeof(out_details->FwUrl) - 1); @@ -1397,29 +2443,51 @@ static bool parse_update_details(const char *update_details_str, sizeof(out_details->DelayDownload) - 1); } else if (strcmp(key, "PDRI") == 0) { + /* + * Skip "N/A" -- leave the field as empty string (from memset). + * "N/A" means the daemon has no PDRI version info, which is + * the common case for non-PDRI devices. + */ if (strcmp(value, "N/A") != 0) { strncpy(out_details->PDRIVersion, value, sizeof(out_details->PDRIVersion) - 1); } } else if (strcmp(key, "Peripherals") == 0) { + /* Same N/A handling as PDRI. */ if (strcmp(value, "N/A") != 0) { strncpy(out_details->PeripheralFirmwares, value, sizeof(out_details->PeripheralFirmwares) - 1); } } else if (strcmp(key, "Protocol") == 0 || strcmp(key, "CertBundle") == 0) { - /* These fields exist in daemon format but not in our struct - ignore */ + /* + * These keys exist in the daemon's format but our UpdateDetails + * struct doesn't have fields for them. Log and skip. + * If a future version needs these, add struct fields and + * copy them here. + */ FWUPMGR_INFO("parse_update_details: skipping field '%s'='%s'\n", key, value); } else { - /* Unknown key - log but don't fail */ + /* + * Unknown key -- forward compatibility. If the daemon adds + * new fields in a future version, we log and skip them + * rather than failing. This allows the library to work with + * newer daemons that send extra fields. + */ FWUPMGR_INFO("parse_update_details: unknown key '%s', ignoring\n", key); } + /* Advance to next pipe-separated token. */ token = strtok_r(NULL, "|", &saveptr); } + /* + * Free the working copy. All the data we needed has been copied + * into out_details struct fields (which are char arrays, not pointers + * into work_str). So freeing work_str is safe. + */ free(work_str); FWUPMGR_INFO("parse_update_details: parsed successfully\n"); diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_async_internal.h b/librdkFwupdateMgr/src/rdkFwupdateMgr_async_internal.h index f12de154..89570438 100644 --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_async_internal.h +++ b/librdkFwupdateMgr/src/rdkFwupdateMgr_async_internal.h @@ -71,6 +71,9 @@ extern "C" { #define DBUS_SIGNAL_COMPLETE "CheckForUpdateComplete" #define DBUS_TIMEOUT_MS 5000 +/** D-Bus call timeout for synchronous register/unregister calls (10 seconds) */ +#define DBUS_SYNC_TIMEOUT_MS 10000 + /* ======================================================================== * CALLBACK ENTRY STATE * ======================================================================== */ diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_log.c b/librdkFwupdateMgr/src/rdkFwupdateMgr_log.c deleted file mode 100755 index 9f8862ce..00000000 --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_log.c +++ /dev/null @@ -1,159 +0,0 @@ -/* - * Copyright 2026 Comcast Cable Communications Management, LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * @file rdkFwupdateMgr_log.c - * @brief Logging implementation for librdkFwupdateMgr client library - */ - -#include "rdkFwupdateMgr_log.h" -#include -#include -#include -#include -#include -#include -#include - -/* ======================================================================== - * INTERNAL STATE - * ======================================================================== */ - -static FILE *g_log_file = NULL; -static pthread_mutex_t g_log_mutex = PTHREAD_MUTEX_INITIALIZER; -static int g_log_initialized = 0; - -/* ======================================================================== - * LOGGING IMPLEMENTATION - * ======================================================================== */ - -/** - * @brief Initialize logging - */ -void fwupmgr_log_init(void) -{ - pthread_mutex_lock(&g_log_mutex); - - if (g_log_initialized) { - pthread_mutex_unlock(&g_log_mutex); - return; // Already initialized - } - - // Create log directory if it doesn't exist - mkdir("/opt/logs", 0755); // Ignore error if exists - - // Open log file in append mode - g_log_file = fopen(FWUPMGR_LOG_FILE, "a"); - if (!g_log_file) { - // Fallback to stderr if log file can't be opened - fprintf(stderr, "[%s] WARNING: Cannot open log file %s: %s\n", - FWUPMGR_LOG_MODULE, FWUPMGR_LOG_FILE, strerror(errno)); - fprintf(stderr, "[%s] Logging will go to stderr\n", FWUPMGR_LOG_MODULE); - } else { - // Make log file line-buffered for immediate writes - setlinebuf(g_log_file); - } - - g_log_initialized = 1; - pthread_mutex_unlock(&g_log_mutex); - - // Log initialization message - fwupmgr_log_internal("INFO", "Logging initialized\n"); -} - -/** - * @brief Close logging - */ -void fwupmgr_log_close(void) -{ - pthread_mutex_lock(&g_log_mutex); - - if (!g_log_initialized) { - pthread_mutex_unlock(&g_log_mutex); - return; // Not initialized - } - - if (g_log_file) { - // Write shutdown message directly to avoid deadlock - // (fwupmgr_log_internal would try to lock g_log_mutex again) - time_t now; - struct tm *tm_info; - char timestamp[64]; - - time(&now); - tm_info = localtime(&now); - if (strftime(timestamp, sizeof(timestamp), "%Y-%m-%d %H:%M:%S", tm_info) == 0) { - snprintf(timestamp, sizeof(timestamp), "UNKNOWN-TIME"); - } - - fprintf(g_log_file, "%s [%s] INFO: Logging shutdown\n", - timestamp, FWUPMGR_LOG_MODULE); - fflush(g_log_file); - - fclose(g_log_file); - g_log_file = NULL; - } - - g_log_initialized = 0; - pthread_mutex_unlock(&g_log_mutex); -} - -/** - * @brief Internal logging function with timestamp and thread-safety - */ -void fwupmgr_log_internal(const char *level, const char *format, ...) -{ - time_t now; - struct tm *tm_info; - char timestamp[64]; - va_list args; - FILE *output; - - pthread_mutex_lock(&g_log_mutex); - - // Auto-initialize if not done - if (!g_log_initialized) { - pthread_mutex_unlock(&g_log_mutex); - fwupmgr_log_init(); - pthread_mutex_lock(&g_log_mutex); - } - - // Determine output stream (log file or stderr fallback) - output = g_log_file ? g_log_file : stderr; - - // Get current timestamp - time(&now); - tm_info = localtime(&now); - if (strftime(timestamp, sizeof(timestamp), "%Y-%m-%d %H:%M:%S", tm_info) == 0) { - snprintf(timestamp, sizeof(timestamp), "UNKNOWN-TIME"); - } - - // Write log header: timestamp [MODULE] LEVEL: - fprintf(output, "%s [%s] %s: ", timestamp, FWUPMGR_LOG_MODULE, level); - - // Write log message - va_start(args, format); - vfprintf(output, format, args); - va_end(args); - - // Ensure immediate write - fflush(output); - - pthread_mutex_unlock(&g_log_mutex); -} - diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_log.h b/librdkFwupdateMgr/src/rdkFwupdateMgr_log.h old mode 100755 new mode 100644 index f9012fcf..8f58a3ff --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_log.h +++ b/librdkFwupdateMgr/src/rdkFwupdateMgr_log.h @@ -1,124 +1,86 @@ -/* - * Copyright 2025 Comcast Cable Communications Management, LLC - * - * Licensed under the Apache License, Version 2.0 (the "License"); - * you may not use this file except in compliance with the License. - * You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, - * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. - * See the License for the specific language governing permissions and - * limitations under the License. - * - * SPDX-License-Identifier: Apache-2.0 - */ - -/** - * @file rdkFwupdateMgr_log.h - * @brief Logging macros for librdkFwupdateMgr client library - * - * This header provides logging macros that write to /opt/logs/rdkFwupdateMgr.log - * using the RDK logger infrastructure, similar to SWLOG_* macros used in the daemon. - */ - -#ifndef RDKFWUPDATEMGR_LOG_H -#define RDKFWUPDATEMGR_LOG_H - -#include -#include - -#ifdef __cplusplus -extern "C" { -#endif - -/* ======================================================================== - * LOGGING CONFIGURATION - * ======================================================================== */ - -/** Log file path - same as daemon for consistent logging */ -#define FWUPMGR_LOG_FILE "/opt/logs/rdkFwupdateMgr.log" - -/** Log module name for identification */ -#define FWUPMGR_LOG_MODULE "librdkFwupdateMgr" - -/* ======================================================================== - * LOGGING API - * ======================================================================== */ - -/** - * @brief Initialize logging for the library - * - * Opens the log file for appending. Should be called once at library init. - * Safe to call multiple times (no-op after first call). - */ -void fwupmgr_log_init(void); - -/** - * @brief Close logging resources - * - * Closes the log file. Should be called at library cleanup. - * Safe to call multiple times (no-op if already closed). - */ -void fwupmgr_log_close(void); - -/** - * @brief Internal logging function - * - * @param level Log level string ("INFO", "ERROR", "DEBUG", "WARN") - * @param format Printf-style format string - * @param ... Variable arguments for format string - */ -void fwupmgr_log_internal(const char *level, const char *format, ...); - -/* ======================================================================== - * LOGGING MACROS - Same pattern as SWLOG_* in daemon - * ======================================================================== */ - -/** - * @brief Log informational message - * - * Usage: FWUPMGR_INFO("Registered with handler: %s\n", handler_id); - */ -#define FWUPMGR_INFO(format, ...) \ - fwupmgr_log_internal("INFO", "[%s:%d] " format, __FUNCTION__, __LINE__, ##__VA_ARGS__) - -/** - * @brief Log error message - * - * Usage: FWUPMGR_ERROR("Registration failed: %s\n", error_msg); - */ -#define FWUPMGR_ERROR(format, ...) \ - fwupmgr_log_internal("ERROR", "[%s:%d] " format, __FUNCTION__, __LINE__, ##__VA_ARGS__) - -/** - * @brief Log debug message - * - * Usage: FWUPMGR_DEBUG("D-Bus proxy created: %p\n", proxy); - */ -#define FWUPMGR_DEBUG(format, ...) \ - fwupmgr_log_internal("DEBUG", "[%s:%d] " format, __FUNCTION__, __LINE__, ##__VA_ARGS__) - -/** - * @brief Log warning message - * - * Usage: FWUPMGR_WARN("Daemon not responding, retry recommended\n"); - */ -#define FWUPMGR_WARN(format, ...) \ - fwupmgr_log_internal("WARN", "[%s:%d] " format, __FUNCTION__, __LINE__, ##__VA_ARGS__) - -/** - * @brief Log fatal error message - * - * Usage: FWUPMGR_FATAL("Out of memory, cannot continue\n"); - */ -#define FWUPMGR_FATAL(format, ...) \ - fwupmgr_log_internal("FATAL", "[%s:%d] " format, __FUNCTION__, __LINE__, ##__VA_ARGS__) - -#ifdef __cplusplus -} -#endif - -#endif /* RDKFWUPDATEMGR_LOG_H */ +/* + * Copyright 2025 Comcast Cable Communications Management, LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * + * SPDX-License-Identifier: Apache-2.0 + */ + +/** + * @file rdkFwupdateMgr_log.h + * @brief Logging macros for librdkFwupdateMgr client library + * + * FWUPMGR_* macros log directly with the "LOG.RDK.FWUPMGR" module + * (when RDK_LOGGER is enabled) so that library log lines appear as + * "[FWUPMGR]" in the output clearly distinguishable from daemon + * logs ("[FWUPG]") and common-utility logs ("[COMMONUTILITIES]") + * without any redundant double-tagging. + * + * The hosting application (example_plugin, unit-test harness, etc.) + * is responsible for calling log_init() before using this library + * and log_exit() on shutdown. The library does NOT own the log + * lifecycle. + * + * Usage: + * FWUPMGR_INFO("Registered with handler: %s\n", handler_id); + * FWUPMGR_ERROR("Registration failed: %s\n", error_msg); + * FWUPMGR_DEBUG("D-Bus proxy created: %p\n", proxy); + */ + +#ifndef RDKFWUPDATEMGR_LOG_H +#define RDKFWUPDATEMGR_LOG_H + +#include "rdkv_cdl_log_wrapper.h" /* SWLOG_*, log_init(), log_exit() */ + +#ifdef __cplusplus +extern "C" { +#endif + +/* ======================================================================== + * Library code uses FWUPMGR_* macros - logs as [FWUPMGR] + * Example app defines EXAMPLE_* macros - logs as [EXAMPLE] + * ======================================================================== */ + +#if defined(RDK_LOGGER) +#include "rdk_debug.h" + +/* Generic base macro callers provide their own module name */ +#define FWUPMGR_LOG(level, module, format, ...) \ + RDK_LOG(level, module, format, ##__VA_ARGS__) + +/* Default library macros use LOG.RDK.FWUPMGR */ +#define FWUPMGR_TRACE(format, ...) FWUPMGR_LOG(RDK_LOG_TRACE1, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) +#define FWUPMGR_DEBUG(format, ...) FWUPMGR_LOG(RDK_LOG_DEBUG, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) +#define FWUPMGR_INFO(format, ...) FWUPMGR_LOG(RDK_LOG_INFO, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) +#define FWUPMGR_WARN(format, ...) FWUPMGR_LOG(RDK_LOG_WARN, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) +#define FWUPMGR_ERROR(format, ...) FWUPMGR_LOG(RDK_LOG_ERROR, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) +#define FWUPMGR_FATAL(format, ...) FWUPMGR_LOG(RDK_LOG_FATAL, "LOG.RDK.FWUPMGR", format, ##__VA_ARGS__) + +#else + + +/* Default library macros */ +#define FWUPMGR_TRACE(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) +#define FWUPMGR_DEBUG(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) +#define FWUPMGR_INFO(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) +#define FWUPMGR_WARN(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) +#define FWUPMGR_ERROR(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) +#define FWUPMGR_FATAL(FORMAT...) FWUPMGR_LOG(FWUPMGR_LOG_INFO, "FWUPMGR", FORMAT) + +#endif + +#ifdef __cplusplus +} +#endif + +#endif /* RDKFWUPDATEMGR_LOG_H */ diff --git a/librdkFwupdateMgr/src/rdkFwupdateMgr_process.c b/librdkFwupdateMgr/src/rdkFwupdateMgr_process.c index 8e3c27ae..13717ecc 100755 --- a/librdkFwupdateMgr/src/rdkFwupdateMgr_process.c +++ b/librdkFwupdateMgr/src/rdkFwupdateMgr_process.c @@ -61,7 +61,7 @@ * * ERROR HANDLING: * =============== - * - All errors logged via fprintf(stderr) for visibility + * - All errors logged via FWUPMGR_* macros (rdkv_cdl_log_wrapper backend) * - NULL checks on all pointer parameters * - D-Bus errors caught and handled gracefully * - Registration failures return NULL (safe to check) @@ -69,6 +69,7 @@ */ #include "rdkFwupdateMgr_client.h" +#include "rdkFwupdateMgr_async_internal.h" #include "rdkFwupdateMgr_log.h" #include #include @@ -95,8 +96,7 @@ /** Maximum length for library version string */ #define MAX_LIB_VERSION_LEN 64 -/** Default D-Bus call timeout in milliseconds (10 seconds) */ -#define DBUS_TIMEOUT_MS 10000 +/* DBUS_SYNC_TIMEOUT_MS is defined in rdkFwupdateMgr_async_internal.h (10s) */ /* ======================================================================== * INTERNAL CONTEXT STRUCTURE @@ -236,26 +236,93 @@ static bool validate_lib_version(const char *libVersion) return true; } -/* ======================================================================== - * PUBLIC API IMPLEMENTATION - * ======================================================================== */ +/* PUBLIC API IMPLEMENTATION */ -/** - * @brief Register a process with the firmware update daemon +/* + * registerProcess - Register a client process with the firmware update daemon. * - * See rdkFwupdateMgr_process.h for full API documentation. + * OVERVIEW * - * IMPLEMENTATION NOTES: - * - Creates D-Bus proxy on-demand (no persistent connection) - * - Synchronous D-Bus call (blocks until daemon responds) - * - Timeout: 10 seconds (configurable via DBUS_TIMEOUT_MS) - * - Returns string handle (handler_id as decimal string) + * PURPOSE: + * This is the MANDATORY first call before using any other library API. + * It establishes a session with the rdkFwupdateMgr daemon by: + * 1. Sending the client's name and version to the daemon via D-Bus + * 2. Receiving a unique numeric handler_id from the daemon + * 3. Converting that ID into a string "handle" returned to caller + * 4. Spinning up the library's internal async engine (BG thread, + * callback registries, D-Bus signal subscriptions) * - * ERROR HANDLING: - * - Input validation: NULL/empty checks, length checks - * - D-Bus errors: Connection failures, daemon errors - * - Memory allocation failures - * - All errors return NULL with descriptive stderr messages + * WHAT THE CALLER GETS BACK: + * A FirmwareInterfaceHandle (which is just a typedef for char*). + * Example: "1", "42", "1023" -- it's the daemon-assigned handler_id + * as a decimal string. This handle must be passed to ALL subsequent + * API calls (checkForUpdate, downloadFirmware, updateFirmware) and + * eventually to unregisterProcess() to clean up. + * + * THREADING MODEL: + * - This function runs entirely on the CALLER'S thread + * - It BLOCKS (synchronous D-Bus call) for up to DBUS_SYNC_TIMEOUT_MS (10s) + * - At the end, it spawns a background thread for signal reception + * - After return: 2 threads exist (caller's + library BG thread) + * + * D-BUS WIRE PROTOCOL: + * Method: "RegisterProcess" + * Input: GVariant type "(ss)" -- two strings: processName, libVersion + * Output: GVariant type "(t)" -- one uint64: handler_id + * The call creates an EPHEMERAL D-Bus connection (new connection each + * time, destroyed after use). The daemon identifies us by handler_id, + * NOT by D-Bus sender address. + * + * MEMORY CONTRACT: + * - Library OWNS the returned handle (malloc'd internally) + * - Caller must NEVER free() it directly + * - Caller must call unregisterProcess(handle) to release it + * + * POSSIBLE RETURN VALUES: + * Non-NULL string -- Success. Use this handle for all subsequent calls. + * NULL -- Failure. Check logs. Daemon might not be running. + * + * EXECUTION FLOW (step numbers match code comments below): + * + * [1] Log entry + parameter echo + * [2] Validate processName (NULL? empty? too long?) + * [3] Validate libVersion (NULL? too long?) + * [4] Create ephemeral D-Bus proxy to daemon + * [5] Call "RegisterProcess" method synchronously (BLOCKS here) + * [6] Extract handler_id (uint64) from daemon's reply + * [7] Allocate 32-byte string buffer on heap + * [8] Convert handler_id to decimal string + * [9] Initialize internal async engine: + * - 3 callback registries (check, download, update) + * - 4 mutexes + * - 1 background thread (subscribes to D-Bus signals) + * [10] Return handle to caller + * + * @param processName A human-readable name identifying this client process. + * Must be non-NULL, non-empty, max 256 chars. + * Examples: "example_plugin", "tr069_agent", "webui_service" + * The daemon enforces UNIQUENESS -- two different processes + * cannot register with the same name simultaneously. + * + * @param libVersion The version string of this library the client was built + * against. Must be non-NULL, max 64 chars (empty string OK). + * Typically pass the LIB_VERSION macro from the public header + * (currently "1.0.0"). Used by daemon for compatibility tracking. + * + * @return FirmwareInterfaceHandle (char*) on success -- the session handle. + * NULL on any failure (validation, D-Bus, daemon rejection, OOM). + * + * Note: This function is THREAD-SAFE for concurrent calls with different + * process names. Do NOT call it twice with the same processName -- + * the daemon will reject the second registration. + * + * Warning: After this returns non-NULL, a background thread is running. + * You MUST call unregisterProcess() before process exit, or the + * thread will be forcibly killed by the OS (potential resource leak). + * + * See also: unregisterProcess() -- The cleanup counterpart to this function. + * See also: internal_system_init() -- The async engine startup called at the end. + * See also: checkForUpdate() -- First API you'd typically call after registration. */ FirmwareInterfaceHandle registerProcess(const char *processName, const char *libVersion) { @@ -265,40 +332,41 @@ FirmwareInterfaceHandle registerProcess(const char *processName, const char *lib guint64 handler_id = 0; char *handle_str = NULL; + /* Log entry */ FWUPMGR_INFO("registerProcess() called\n"); FWUPMGR_INFO(" processName: '%s'\n", processName ? processName : "NULL"); FWUPMGR_INFO(" libVersion: '%s'\n", libVersion ? libVersion : "NULL"); - // Validate inputs + /* Validate processName */ if (!validate_process_name(processName)) { return NULL; } + /* Validate libVersion */ if (!validate_lib_version(libVersion)) { return NULL; } - // Create D-Bus proxy + /* Create ephemeral D-Bus proxy */ proxy = create_dbus_proxy(&error); if (!proxy) { - // Error already logged by create_dbus_proxy if (error) { g_error_free(error); } return NULL; } - fprintf(stderr, "[rdkFwupdateMgr] D-Bus proxy created successfully\n"); + FWUPMGR_INFO("D-Bus proxy created successfully\n"); - // Call RegisterProcess D-Bus method - fprintf(stderr, "[rdkFwupdateMgr] Calling RegisterProcess D-Bus method...\n"); + /* Synchronous D-Bus call: RegisterProcess(processName, libVersion) → handler_id */ + FWUPMGR_INFO("Calling RegisterProcess D-Bus method...\n"); result = g_dbus_proxy_call_sync( proxy, "RegisterProcess", g_variant_new("(ss)", processName, libVersion), G_DBUS_CALL_FLAGS_NONE, - DBUS_TIMEOUT_MS, - NULL, // GCancellable + DBUS_SYNC_TIMEOUT_MS, + NULL, /* GCancellable */ &error ); @@ -310,7 +378,7 @@ FirmwareInterfaceHandle registerProcess(const char *processName, const char *lib return NULL; } - // Extract handler_id from result + /* Extract handler_id from daemon reply "(t)" and free D-Bus resources */ g_variant_get(result, "(t)", &handler_id); g_variant_unref(result); g_object_unref(proxy); @@ -318,17 +386,17 @@ FirmwareInterfaceHandle registerProcess(const char *processName, const char *lib FWUPMGR_INFO("Registration successful\n"); FWUPMGR_INFO(" handler_id: %"G_GUINT64_FORMAT"\n", handler_id); - // Convert handler_id to string (this becomes the handle) - handle_str = (char*)malloc(32); // Enough for uint64 as decimal string + /* Allocate handle string (32 bytes: enough for max uint64 decimal) */ + handle_str = (char*)malloc(32); if (!handle_str) { FWUPMGR_ERROR("Failed to allocate memory for handle\n"); - // ⚠️ CRITICAL: Registration succeeded on daemon, but we can't return handle. - // Must unregister to prevent resource leak on daemon side. + /* CRITICAL: Registration succeeded on daemon, but we can't return handle. + * Must unregister to prevent resource leak on daemon side. */ FWUPMGR_ERROR("Attempting best-effort cleanup: UnregisterProcess(%" PRIu64 ")\n", handler_id); - // Create new proxy for cleanup call (previous one was already freed) + /* Create new proxy for cleanup call (previous one was already freed) */ GError *cleanup_error = NULL; GDBusProxy *cleanup_proxy = create_dbus_proxy(&cleanup_error); if (cleanup_proxy) { @@ -337,7 +405,7 @@ FirmwareInterfaceHandle registerProcess(const char *processName, const char *lib "UnregisterProcess", g_variant_new("(t)", handler_id), G_DBUS_CALL_FLAGS_NONE, - DBUS_TIMEOUT_MS, + DBUS_SYNC_TIMEOUT_MS, NULL, &cleanup_error ); @@ -359,28 +427,136 @@ FirmwareInterfaceHandle registerProcess(const char *processName, const char *lib return NULL; } - //snprintf(handle_str, 32, "" %PRIu64, handler_id); + /* Convert handler_id to decimal string */ snprintf(handle_str, 32, "%" PRIu64, handler_id); FWUPMGR_INFO("Handle created: '%s'\n", handle_str); + /* Initialize async engine: registries, mutexes, BG thread */ + FWUPMGR_INFO("=== rdkFwupdateMgr Creating thread for listen ===\n"); + if (internal_system_init() != 0) { + FWUPMGR_ERROR("rdkFwupdateMgr_lib_init: internal_system_init FAILED\n"); + GError *cleanup_error = NULL; + GDBusProxy *cleanup_proxy = create_dbus_proxy(&cleanup_error); + if (cleanup_proxy) { + GVariant *cleanup_result = g_dbus_proxy_call_sync( + cleanup_proxy, + "UnregisterProcess", + g_variant_new("(t)", handler_id), + G_DBUS_CALL_FLAGS_NONE, + DBUS_SYNC_TIMEOUT_MS, + NULL, + &cleanup_error + ); + if (cleanup_result) { + FWUPMGR_INFO("Cleanup successful: process unregistered\n"); + g_variant_unref(cleanup_result); + } else { + FWUPMGR_ERROR("Cleanup failed: %s (registration may be leaked)\n", + cleanup_error ? cleanup_error->message : "unknown"); + if (cleanup_error) g_error_free(cleanup_error); + } + g_object_unref(cleanup_proxy); + } else { + FWUPMGR_ERROR("Cleanup proxy creation failed (registration leaked)\n"); + if (cleanup_error) g_error_free(cleanup_error); + } + + free(handle_str); + return NULL; + } + FWUPMGR_INFO("=== rdkFwupdateMgr Creating thread for listen successful ===\n"); + + /* Return handle to caller */ return (FirmwareInterfaceHandle)handle_str; } -/** - * @brief Unregister a previously registered process +/* + * unregisterProcess - Tear down the library and deregister from the daemon. * - * See rdkFwupdateMgr_process.h for full API documentation. + * OVERVIEW * - * IMPLEMENTATION NOTES: - * - Best-effort cleanup (errors are logged but not propagated) - * - Frees handle memory regardless of D-Bus call success - * - Idempotent: Safe to call with NULL handle (no-op) - * - Daemon may already have removed the registration (connection lost) + * PURPOSE: + * This is the MANDATORY last call before the client process exits. + * It is the mirror image of registerProcess(). Where registerProcess() + * builds up the machinery (thread, mutexes, registries, D-Bus connection), + * this function tears it ALL down in the correct order. * - * ERROR HANDLING: - * - NULL handle: No-op, returns immediately - * - D-Bus errors: Logged to stderr, but cleanup continues - * - Memory freed regardless of D-Bus call success + * Specifically, it does three things: + * 1. Shuts down the library's internal async engine (kills the BG thread, + * destroys all 4 mutexes, frees all 3 callback registries) + * 2. Tells the daemon "I'm leaving" via a D-Bus call (best-effort) + * 3. Frees the handle string that registerProcess() malloc'd + * + * WHY THE ORDER MATTERS: + * LOCAL cleanup (step 1) happens BEFORE the D-Bus call (step 2). + * This is deliberate: + * - After we tell the daemon "I'm leaving", it stops sending signals. + * - If the BG thread was still alive, it would sit forever in + * g_main_loop_run() waiting for signals that never come. + * - pthread_join() would block indefinitely. + * So we kill the BG thread FIRST, then tell the daemon. + * The D-Bus call is best-effort anyway -- if it fails, the daemon + * eventually cleans up stale registrations on its own. + * + * THREADING MODEL: + * - This function runs on the CALLER'S thread (main thread of example_app) + * - BEFORE this call: 2 threads (caller's + BG thread) + * - AFTER this call: 1 thread (caller's only -- BG thread joined and dead) + * - The D-Bus call (step 2) is synchronous, blocks up to 5 seconds + * + * D-BUS WIRE PROTOCOL: + * Method: "UnregisterProcess" + * Input: GVariant type "(t)" -- one uint64: handler_id + * Output: GVariant type "(b)" -- one boolean: success + * This creates an EPHEMERAL D-Bus connection (different sender ID from + * every other call). The daemon matches by handler_id, not sender. + * + * MEMORY CONTRACT: + * - This function FREES the handle string (the pointer becomes invalid) + * - Caller must NOT use the handle after this call returns + * - Caller should set their local copy to NULL as defensive practice + * + * RETURN VALUE: + * void -- this function always succeeds from the caller's perspective. + * All errors are logged but swallowed. Best-effort cleanup. + * + * SAFE TO CALL WITH NULL: + * Passing NULL is a no-op. This allows the caller to do: + * unregisterProcess(handle); (where handle might be NULL) + * without needing a NULL check at every call site. + * + * EXECUTION FLOW (step numbers match code comments below): + * + * [1] NULL check -- if NULL, return immediately (no-op) + * [2] Parse handle string "1" to uint64 handler_id = 1 + * Uses strtoull() with strict validation (reject garbage) + * [3] internal_system_deinit() -- tear down the async engine: + * - g_main_loop_quit() wakes BG thread from g_main_loop_run() + * - pthread_join() waits for BG thread to exit + * - Free GMainLoop and GMainContext + * - Destroy g_bg_thread.mutex + * - Free download registry (handle_keys + mutex) + * - Free update registry (handle_keys + mutex) + * - Free check registry (handle_keys + mutex) + * After this: 1 thread, 0 mutexes, 0 D-Bus connections + * [4] Create ephemeral D-Bus proxy (best-effort) + * [5] Call "UnregisterProcess" on daemon (best-effort, blocks up to 10s) + * [6] Extract success boolean from daemon reply + * [7] free(handler) -- always, regardless of D-Bus outcome + * + * @param handler The handle returned by registerProcess(). May be NULL. + * After this call returns, this pointer is INVALID (freed). + * + * Note: This function is deliberately tolerant of errors. Every failure + * path still frees the handle and returns cleanly. The daemon's + * ProcessInfo entry may be orphaned if the D-Bus call fails, but + * that is the daemon's responsibility to clean up. + * + * Warning: Do NOT call this from multiple threads with the same handle. + * Do NOT use the handle after this call returns. + * + * See also: registerProcess() -- the setup counterpart to this function. + * See also: internal_system_deinit() -- the async engine teardown. */ void unregisterProcess(FirmwareInterfaceHandle handler) { @@ -390,7 +566,7 @@ void unregisterProcess(FirmwareInterfaceHandle handler) guint64 handler_id = 0; gboolean success = FALSE; - // NULL check: Safe to unregister NULL handle (no-op) + /* NULL handle is a safe no-op */ if (!handler) { FWUPMGR_INFO("unregisterProcess() called with NULL handle (no-op)\n"); return; @@ -399,29 +575,23 @@ void unregisterProcess(FirmwareInterfaceHandle handler) FWUPMGR_INFO("unregisterProcess() called\n"); FWUPMGR_INFO(" handle: '%s'\n", handler); - // Parse handler_id from string handle with strict validation - // Reject handles with leading/trailing whitespace or garbage characters - // Valid examples: "12345", "0" - // Invalid examples: "123abc", " 123", "123 ", "", "abc" + /* Parse handle string to uint64 handler_id (strict validation) */ errno = 0; char *endptr = NULL; handler_id = strtoull(handler, &endptr, 10); - - // Validation checks: - // 1. errno != 0: overflow or underflow occurred - // 2. endptr == handler: no digits were parsed (e.g., "abc") - // 3. *endptr != '\0': garbage after number (e.g., "123abc") - // 4. handler_id == 0: could be valid "0" or invalid input - check endptr + if (errno != 0) { FWUPMGR_ERROR("Invalid handle: numeric overflow/underflow in '%s'\n", handler); free(handler); return; } + if (endptr == handler) { FWUPMGR_ERROR("Invalid handle: no digits found in '%s'\n", handler); free(handler); return; } + if (*endptr != '\0') { FWUPMGR_ERROR("Invalid handle: garbage characters after number in '%s' " "(parsed %" PRIu64 ", but '%s' remains)\n", @@ -429,6 +599,7 @@ void unregisterProcess(FirmwareInterfaceHandle handler) free(handler); return; } + if (handler_id == 0) { FWUPMGR_ERROR("Invalid handle: handler_id cannot be 0\n"); free(handler); @@ -437,7 +608,12 @@ void unregisterProcess(FirmwareInterfaceHandle handler) FWUPMGR_INFO(" handler_id: %"G_GUINT64_FORMAT"\n", handler_id); - // Create D-Bus proxy + /* Tear down async engine: kill BG thread, destroy mutexes, free registries */ + FWUPMGR_INFO("=== rdkFwupdateMgr destroy thread unloading ===\n"); + internal_system_deinit(); + FWUPMGR_INFO("=== rdkFwupdateMgr destroy thread ===\n"); + + /*Create D-Bus proxy (best-effort -- local cleanup already done) */ proxy = create_dbus_proxy(&error); if (!proxy) { FWUPMGR_WARN("Failed to create D-Bus proxy for unregister\n"); @@ -445,35 +621,35 @@ void unregisterProcess(FirmwareInterfaceHandle handler) FWUPMGR_WARN(" Error: %s\n", error->message); g_error_free(error); } - // Continue with cleanup even if D-Bus call fails free(handler); return; } - // Call UnregisterProcess D-Bus method + /* Best-effort D-Bus call: UnregisterProcess(handler_id) */ FWUPMGR_INFO("Calling UnregisterProcess D-Bus method...\n"); result = g_dbus_proxy_call_sync( proxy, "UnregisterProcess", g_variant_new("(t)", handler_id), G_DBUS_CALL_FLAGS_NONE, - DBUS_TIMEOUT_MS, - NULL, // GCancellable + DBUS_SYNC_TIMEOUT_MS, + NULL, /* GCancellable */ &error ); if (!result) { FWUPMGR_WARN("UnregisterProcess D-Bus call failed: %s\n", - error->message); + error ? error->message : "unknown error (GError not set)"); FWUPMGR_WARN(" (This is OK if daemon already cleaned up)\n"); - g_error_free(error); + if (error) { + g_error_free(error); + } g_object_unref(proxy); - // Continue with local cleanup free(handler); return; } - // Extract success flag from result + /* Extract success flag from daemon reply "(b)" */ g_variant_get(result, "(b)", &success); g_variant_unref(result); g_object_unref(proxy); @@ -485,7 +661,7 @@ void unregisterProcess(FirmwareInterfaceHandle handler) FWUPMGR_WARN(" (Handler may have already been unregistered)\n"); } - // Free handle memory (always, regardless of D-Bus call success) + /* Free the handle string (caller must not use handle after this) */ free(handler); FWUPMGR_INFO("Handle memory freed\n"); } diff --git a/src/dbus/rdkFwupdateMgr_handlers.c b/src/dbus/rdkFwupdateMgr_handlers.c index d80af84d..d27eadcf 100644 --- a/src/dbus/rdkFwupdateMgr_handlers.c +++ b/src/dbus/rdkFwupdateMgr_handlers.c @@ -473,11 +473,11 @@ static int fetch_xconf_firmware_info( XCONFRES *pResponse, int server_type, int xconf_context.dwlloc = &DwnLoc; xconf_context.pPostFields = pJSONStr; - //Have to revist these vars once - MADHU + Rfc_t local_rfc_list = {0}; getRFCSettings(&local_rfc_list); // Read actual RFC settings from system - const char *local_immed_reboot_flag = "false"; // Default daemon setting + const char *local_immed_reboot_flag = "NA"; // Keeping it as NA as default setting to make it work in Throttle enable mode. int local_delay_dwnl = 0; // Default daemon setting const char *local_lastrun = "0"; // Default daemon setting char *local_disableStatsUpdate = "false"; // Default daemon setting diff --git a/src/dbus/rdkv_dbus_server.c b/src/dbus/rdkv_dbus_server.c index 1eaa2adf..d70bfaa1 100644 --- a/src/dbus/rdkv_dbus_server.c +++ b/src/dbus/rdkv_dbus_server.c @@ -399,27 +399,22 @@ static guint64 add_process_to_tracking(const gchar *process_name, * @brief Remove a process from the tracking system. * * Called when client invokes UnregisterProcess. Frees associated ProcessInfo. - * Validates that the requesting client is the owner of the handler_id. * * @param handler_id Handler ID to remove - * @param sender_id D-Bus sender ID of the requesting client - * @return TRUE if found and removed, FALSE if not found or access denied + * @param sender_id D-Bus sender ID of the requesting client; currently unused but + * retained to keep this internal helper aligned with its caller context and to + * allow future sender-based validation or auditing. + * @return TRUE if found and removed, FALSE if not found */ static gboolean remove_process_from_tracking(guint64 handler_id, const gchar *sender_id) { + (void)sender_id; // Intentionally unused for now; reserved for possible sender-aware checks ProcessInfo *info = g_hash_table_lookup(registered_processes, GINT_TO_POINTER(handler_id)); if (!info) { SWLOG_INFO("[PROCESS_TRACKING] Handler %"G_GUINT64_FORMAT" not found\n", handler_id); return FALSE; } - // SECURITY: Validate that the requesting client owns this handler_id - if (g_strcmp0(info->sender_id, sender_id) != 0) { - SWLOG_ERROR("[PROCESS_TRACKING] Access denied: Handler %"G_GUINT64_FORMAT" owned by '%s', but '%s' attempted to unregister\n", - handler_id, info->sender_id, sender_id); - return FALSE; - } - SWLOG_INFO("[PROCESS_TRACKING] Removing: %s (handler: %"G_GUINT64_FORMAT")\n", info->process_name, handler_id); g_hash_table_remove(registered_processes, GINT_TO_POINTER(handler_id)); SWLOG_INFO("[PROCESS_TRACKING] Total registered: %d\n", g_hash_table_size(registered_processes)); @@ -1900,17 +1895,7 @@ static void process_app_request(GDBusConnection *rdkv_conn_dbus, ProcessInfo *process_info = g_hash_table_lookup(registered_processes, GINT_TO_POINTER(handler)); const gchar *process_name = process_info ? process_info->process_name : "UNKNOWN"; - // Validate ownership before attempting removal - if (process_info && g_strcmp0(process_info->sender_id, rdkv_req_caller_id) != 0) { - SWLOG_ERROR("[UNREGISTER] Access denied: Handler %"G_GUINT64_FORMAT" (process: %s) owned by '%s', but '%s' attempted to unregister\n", - handler, process_name, process_info->sender_id, rdkv_req_caller_id); - g_dbus_method_invocation_return_error(resp_ctx, - G_DBUS_ERROR, G_DBUS_ERROR_ACCESS_DENIED, - "Unregistration denied: Handler owned by different client"); - return; - } - - // Remove from tracking system (this will double-check ownership) + // Remove from tracking system SWLOG_INFO("[UNREGISTER] Attempting to remove process '%s' from tracking...\n", process_name); if (remove_process_from_tracking(handler, rdkv_req_caller_id)) { SWLOG_INFO("[UNREGISTER] SUCCESS: Process '%s' unregistered successfully!\n", process_name); diff --git a/src/rdkFwupdateMgr.c b/src/rdkFwupdateMgr.c index ba589c29..5b23cdba 100644 --- a/src/rdkFwupdateMgr.c +++ b/src/rdkFwupdateMgr.c @@ -1243,32 +1243,39 @@ int main(int argc, char *argv[]) { SWLOG_INFO("XCONF Download Fail\n"); } */ + } + else if(init_validate_status == INITIAL_VALIDATION_DWNL_COMPLETED){ + /** + * A previous firmware download+flash already completed + * (/tmp/fw_preparing_to_reboot was present). + * The file has been cleaned up by initialValidation(). + * initialValidation() is also responsible for emitting + * the MAINT_FWDOWNLOAD_COMPLETE event for this case. + * In daemon mode, we transition to IDLE and wait for + * the pending reboot or the next D-Bus request. + */ + SWLOG_INFO("Software Update already completed (pending reboot). " + "Transitioning to IDLE.\n"); + currentState = STATE_IDLE; + } + else if(init_validate_status == INITIAL_VALIDATION_DWNL_INPROGRESS){ + /** + * Another instance is currently downloading firmware. + * In the daemon model, transition to IDLE and wait. + * The in-progress download will complete independently. + */ + SWLOG_INFO("Firmware download already in progress by another process. " + "Transitioning to IDLE.\n"); + if (0 == (strncmp(device_info.maint_status, "true", 4))) { + eventManager("MaintenanceMGR", MAINT_FWDOWNLOAD_INPROGRESS); + } + currentState = STATE_IDLE; } else{ - SWLOG_ERROR("Initial validation failed\n"); + /* INITIAL_VALIDATION_FAIL or unknown status */ + SWLOG_ERROR("Initial validation failed (status=%d)\n", init_validate_status); goto cleanup_and_exit; } - /*this is for sending the intermediate updates back to apps and */ - /* - if (init_validate_status == INITIAL_VALIDATION_DWNL_INPROGRESS){ - if (!(strncmp(device_info.maint_status, "true", 4))) { - eventManager("MaintenanceMGR", MAINT_FWDOWNLOAD_INPROGRESS); //Sending status to maintenance manager - } - }else if(init_validate_status == INITIAL_VALIDATION_DWNL_COMPLETED) { - SWLOG_INFO("Software Update is completed by AS/EPG, Exiting from firmware download.\n"); - }else if ((ret_curl_code != 0) || (json_res != 0)) { - if (!(strncmp(device_info.maint_status, "true", 4))) { - eventManager("MaintenanceMGR", MAINT_FWDOWNLOAD_ERROR); //Sending status to maintenance manager - } - if (trigger_type == 6) { - unsetStateRed(); - } - }else { - if (!(strncmp(device_info.maint_status, "true", 4))) { - eventManager("MaintenanceMGR", MAINT_FWDOWNLOAD_COMPLETE); //Sending status to maintenance manager - } - } - */ break; case STATE_IDLE: /** diff --git a/src/rdkv_upgrade.c b/src/rdkv_upgrade.c index 71fe70b4..a23cc8ba 100755 --- a/src/rdkv_upgrade.c +++ b/src/rdkv_upgrade.c @@ -983,9 +983,6 @@ int downloadFile( } if ((1 == (isThrottleEnabled(device_info->dev_name, immed_reboot_flag, app_mode)))) { - /* Coverity fix: NO_EFFECT - rfc_throttle is a char array, not a pointer. - * Removed redundant "!= NULL" check. Only check for non-empty string. - * Ensure rfc_list is valid before dereferencing. */ if (rfc_list != NULL && rfc_list->rfc_throttle[0] != '\0' && 0 == (strncmp(rfc_list->rfc_throttle, "true", 4))) { max_dwnl_speed = atoi(rfc_list->rfc_topspeed);