Skip to content

PR : gh #96:Improved documentation for control plane apis#97

Merged
kanjoe24 merged 7 commits intodevelopfrom
feature/gh96-transfer-ownership
Oct 21, 2025
Merged

PR : gh #96:Improved documentation for control plane apis#97
kanjoe24 merged 7 commits intodevelopfrom
feature/gh96-transfer-ownership

Conversation

@kanjoe24
Copy link
Contributor

@kanjoe24 kanjoe24 commented Sep 24, 2025

Currently its control plane's responsibility to free the kvp instance after callback is called.
The client on the other side queues these messages and processes it at its own time. So by the time it processes, the instance is freed.

By transferring the ownership to the client, client can process at its own time and free the instance.

NOTE : #97 (comment)

@kanjoe24 kanjoe24 requested a review from a team as a code owner September 24, 2025 10:29
@kanjoe24 kanjoe24 linked an issue Sep 24, 2025 that may be closed by this pull request
@kanjoe24 kanjoe24 self-assigned this Sep 24, 2025
@kanjoe24 kanjoe24 requested review from Ulrond and bhanucbp September 24, 2025 10:30
Copy link
Contributor

@Ulrond Ulrond Sep 24, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we need to make it clear, that the memory is internal and will be freed.. If due to the clients implementation they want to offline process the data, then they must copy it for that purpose.

/**
 * @typedef ut_control_callback_t
 * @brief Function pointer type for control plane message callbacks.
 *
 * The callback is invoked synchronously when a registered message key is
 * received. Its primary purpose is to notify the caller that the key has been
 * triggered. The accompanying data is provided for informational use and
 * transient decision-making within the callback.
 *
 * The data associated with the `instance` parameter is only valid for the
 * duration of the callback execution. In most cases, it is not expected to be
 * copied, as it is intended for on-the-spot processing. However, if the caller
 * requires the data for further use outside the callback, they may copy it
 * into their own storage before returning.
 *
 * Memory management of the `instance` is handled internally. The caller must
 * not free it, and must not use it after the callback has returned.
 *
 * @param key - Null-terminated string representing the message key that
 *              triggered the callback.
 * @param instance - Pointer to a key-value pair (`ut_kvp_instance_t`) instance
 *                   containing the message data. Valid only during the callback.
 * @param userData - User-provided pointer passed during callback registration.
 */
typedef void (*ut_control_callback_t)(
    char *key,
    ut_kvp_instance_t *instance,
    void *userData
);

But I would question why they should be doing that.

/**
* @brief Registers a synchronous callback function for a specific message key.
*
* The callback is invoked immediately when the specified key is received.
* The data provided to the callback is only valid for the duration of the
* callback execution. If the user wishes to retain or process the data
* after the callback returns, they must copy it to their own storage.
*
* The memory associated with the callback data is automatically released
* once the callback returns. It is NOT the responsibility of the caller
* to free this memory. However, it is the responsibility of the caller
* to ensure they do not access or use the callback data after the
* callback has returned.
*
* @param pInstance - Handle to the control plane instance.
* @param key - Null-terminated string representing the message key to trigger the callback.
* @param callbackFunction - Callback function to be invoked synchronously when the key is received.
* @param userData - User-provided handle passed back to the callback.
* @returns Status of the registration operation (`ut_control_plane_status_t`).
* @retval UT_CONTROL_PLANE_STATUS_OK - Success.
* @retval UT_CONTROL_PLANE_STATUS_INVALID_HANDLE - Invalid control plane instance handle.
* @retval UT_CONTROL_PLANE_STATUS_INVALID_PARAM - Invalid parameter passed.
* @retval UT_CONTROL_PLANE_STATUS_CALLBACK_LIST_FULL - Callback list is full.
*/
ut_control_plane_status_t UT_ControlPlane_RegisterCallbackOnMessage(
    ut_controlPlane_instance_t *pInstance,
    char *key,
    ut_control_callback_t callbackFunction,
    void *userData
);
``

@kanjoe24 kanjoe24 requested a review from Ulrond September 25, 2025 11:34
@kanjoe24
Copy link
Contributor Author

Note: This PR no longer transfers the ownership. We have however improved documentation to make it clear as to how control plane APIs needs to be used.

@kanjoe24 kanjoe24 changed the title PR : gh #96:Transfer the responsibility of freeing instance to the client via callback PR : gh #96:Improved documentation for control plane apis Sep 25, 2025
@kanjoe24 kanjoe24 requested a review from Ulrond September 29, 2025 12:06
Ulrond
Ulrond previously approved these changes Oct 8, 2025
Copy link
Contributor

@Ulrond Ulrond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do you have memory leak checking enabled in your testing app? Something to consider if not, when you exit the app it will report any leaks.

CFLAGS="-g -O1 -fno-omit-frame-pointer -fsanitize=address -fsanitize=leak"
CXXFLAGS="$CFLAGS"
LDFLAGS="-fsanitize=address -fsanitize=leak"

@Ulrond
Copy link
Contributor

Ulrond commented Oct 8, 2025

also for review you should remove all old test runs, and only include the final one.

@kanjoe24
Copy link
Contributor Author

Attaching test logs after enabling address and leak sanitizer flags in tests makefile.
SanitizerEnabled-TestLogs-Ut-control-tests.rtf

Copy link
Contributor

@Ulrond Ulrond left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good.

@kanjoe24
Copy link
Contributor Author

Raised ticket : #100

@kanjoe24
Copy link
Contributor Author

Release script logs:

./release-test-script-ut-control.sh -t feature/gh96-transfer-ownership
==========================================================
RESULTS for kirkstone_arm 
On the branch feature/gh96-transfer-ownership. PASS
build/arm/curl/lib/libcurl.a exists. PASS
build/arm/openssl/lib/libssl.a exists. PASS 
CMake host binary does not exist. PASS 
==========================================================
==========================================================
RESULTS for ubuntu 
On the branch feature/gh96-transfer-ownership. PASS
build/linux/curl/lib/libcurl.a exists. PASS
Openssl static lib does not exist. PASS 
CMake host binary does not exist. PASS 
==========================================================
==========================================================
RESULTS for VM-SYNC 
On the branch feature/gh96-transfer-ownership. PASS
build/linux/curl/lib/libcurl.a exists. PASS
build/linux/openssl/lib/libssl.a exists. PASS 
host-tools/CMake-3.30.0/build/bin/cmake exists. PASS 
==========================================================
==========================================================
RESULTS for dunfell_arm 
On the branch feature/gh96-transfer-ownership. PASS
build/arm/curl/lib/libcurl.a exists. PASS
build/arm/openssl/lib/libssl.a exists. PASS 
CMake host binary does not exist. PASS 
==========================================================
==========================================================
RESULTS for dunfell_linux 
On the branch feature/gh96-transfer-ownership. PASS
CURL static lib does not exist. PASS
Openssl static lib does not exist. PASS 
CMake host binary exists. PASS 
==========================================================
==========================================================
RESULTS for kirkstone_arm 
On the branch feature/gh96-transfer-ownership. PASS
build/arm/curl/lib/libcurl.a exists. PASS
build/arm/openssl/lib/libssl.a exists. PASS 
CMake host binary does not exist. PASS 
==========================================================
==========================================================
RESULTS for kirkstone_linux 
On the branch feature/gh96-transfer-ownership. PASS
build/linux/curl/lib/libcurl.a exists. PASS
Openssl static lib does not exist. PASS 
CMake host binary does not exist. PASS 
==========================================================

@kanjoe24 kanjoe24 merged commit ddada1d into develop Oct 21, 2025
7 checks passed
@kanjoe24 kanjoe24 deleted the feature/gh96-transfer-ownership branch October 21, 2025 08:28
@github-actions github-actions bot locked and limited conversation to collaborators Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Bug: pkvpInstance destroyed in call_callback_on_match

2 participants

Comments