Feature/cord 2025q4#105
Conversation
This reverts commit ec4426f.
XB9-668 : Upstream xb9 specific changes to github repo
Reason for change: replacing PSM APIs with CORD APIs Test Procedure: build and verify Risks: Low Priority: P1 Signed-off-by: mkorav871 madhukrishnasmenon@comcast.com
There was a problem hiding this comment.
Pull request overview
This PR appears to introduce optional CORD-backed persistent storage support in the CCSP base API layer, along with a set of 64-bit portability adjustments (pointer formatting/casting) and voice-component gating in startup/restart scripts.
Changes:
- Add
--enable-cordconfigure flag and implement CORD-backed versions of PSM get/set/delete/enumeration APIs inccsp_base_api.c. - Adjust a number of pointer casts / printf formats under
_64BIT_ARCH_SUPPORT_across TLS, ANSC, and debug utilities. - Gate MTA startup/kill logic on
VOICE_SUPPORTEDin several scripts.
Reviewed changes
Copilot reviewed 17 out of 17 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| source/util_api/tls/components/TlsHandShaker/tls_hso_control.c | Adds _64BIT_ARCH_SUPPORT_ branches around key/IV pointer selection logic. |
| source/util_api/ccsp_msg_bus/ccsp_message_bus.c | Changes int64/uint64 string formatting logic and removes <inttypes.h>. |
| source/util_api/ccsp_msg_bus/ccsp_base_api.c | Adds CORD-enabled implementations for multiple PSM APIs and CORD-backed list/enumeration helpers. |
| source/util_api/ansc/AnscXmlDomParser/ansc_xml_dom_node_base.c | Changes warning formatting under _64BIT_ARCH_SUPPORT_. |
| source/util_api/ansc/AnscPlatform/user_file_io.c | Adjusts file-handle casting for lseek under _64BIT_ARCH_SUPPORT_. |
| source/util_api/ansc/AnscPlatform/ansc_file_io.c | Adjusts file-handle casting for _ansc_get_file_size under _64BIT_ARCH_SUPPORT_. |
| source/util_api/ansc/AnscPlatform/ansc_debug.c | Pointer printing adjusted for 32/64-bit. |
| source/debug_api/ansc_memory.c | Pointer printing adjusted for 32/64-bit in memory tracing/inspection. |
| source/debug_api/ansc_debug.c | Wraps MTA log-module mapping behind NO_MTA_FEATURE_SUPPORT. |
| source/cosa/utilities/bsp_eng/bspeng_co_execute.c | Adjusts handle formatting/casts under _64BIT_ARCH_SUPPORT_. |
| source/cosa/utilities/bsp_eng/bspeng_branch_co_process.c | Changes enum cast used when loading branch data. |
| source/cosa/utilities/AnscObjectArray/AnscUIntArray/ansc_uint_array_co_process.c | Adjusts casts under _64BIT_ARCH_SUPPORT_ for UIntArray storage. |
| source/ccsp/components/common/DataModel/dml/components/DslhObjController/dslh_objco_access.c | Adjusts uint32 syntax comparison under _64BIT_ARCH_SUPPORT_. |
| scripts/cosa_start_rem.sh | Wraps MTA start section with VOICE_SUPPORTED check. |
| scripts/cosa_start_arm.sh | Wraps MTA kill section with VOICE_SUPPORTED check. |
| scripts/ccsp_restart.sh | Wraps MTA kill/start sections with VOICE_SUPPORTED check. |
| configure.ac | Adds --enable-cord flag and WITH_CORD_SUPPORT conditional. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #else | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
There was a problem hiding this comment.
The ternary initializers are currently being cast in a way that applies the cast to the condition (due to operator precedence), producing a pointer value 0/1 that is then used as the ternary condition. This is implementation-defined and obscures the intent. Remove the cast entirely (both branches already have the correct pointer type) or parenthesize the conditional expression so any cast applies to the selected pointer, not to the boolean condition.
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| ANSC_CRYPTO_KEY* pMacSecretC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #else | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
There was a problem hiding this comment.
pCipherIV_C/S are declared as ANSC_CRYPTO_IV* but are cast from ANSC_CRYPTO_KEY*. This hides a type mismatch and can lead to incorrect aliasing/field access if ANSC_CRYPTO_IV is not layout-compatible with ANSC_CRYPTO_KEY. Cast to ANSC_CRYPTO_IV* (or better, remove the cast) so the pointer type matches the target object.
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_IV* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_IV* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_IV* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_IV* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #else | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
There was a problem hiding this comment.
The 64BIT_ARCH_SUPPORT branch uses (ULONG)(condition) casts in a ternary initializer; due to precedence this converts the boolean to a pointer value (0/1) and uses that as the condition. This is implementation-defined and unnecessary—remove the casts or parenthesize so the conditional expression is evaluated normally.
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| ANSC_CRYPTO_KEY* pMacSecretC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #else | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
There was a problem hiding this comment.
pCipherIV_C/S are ANSC_CRYPTO_IV* but are being cast from ANSC_CRYPTO_KEY* in the ternary. This should be an ANSC_CRYPTO_IV* (or no cast) to avoid hiding an actual type mismatch.
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
| if (pItem->pParameterName[len-1] != '.') { | ||
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | ||
| //pRecArray[idxRecArray].InstanceNumber = safe_atou(<last node name, e.g. "123">); | ||
| safe_atou(pItem->pParameterName[len-1], &(pRecArray[idxRecArray].Instance.InstanceNumber)); |
There was a problem hiding this comment.
Instance number parsing uses safe_atou(pItem->pParameterName[len-1], ...), which passes a single character (and not a numeric substring) to the converter. This won't yield the expected instance number. Parse the final node name (substring after the last '.') and convert that full string into InstanceNumber.
| if (pItem->pParameterName[len-1] != '.') { | |
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | |
| //pRecArray[idxRecArray].InstanceNumber = safe_atou(<last node name, e.g. "123">); | |
| safe_atou(pItem->pParameterName[len-1], &(pRecArray[idxRecArray].Instance.InstanceNumber)); | |
| if (len > 0 && pItem->pParameterName[len-1] != '.') { | |
| const char *pInstanceName = strrchr(pItem->pParameterName, '.'); | |
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | |
| if (pInstanceName != NULL) { | |
| pInstanceName++; | |
| } | |
| else { | |
| pInstanceName = pItem->pParameterName; | |
| } | |
| safe_atou(pInstanceName, &(pRecArray[idxRecArray].Instance.InstanceNumber)); |
| return FALSE; | ||
|
|
||
| pBrData->type = (BSP_TEMPLATE_BRANCH_TYPE)ucType; | ||
| pBrData->type = (BSP_TEMPLATE_OPERATOR)ucType; |
There was a problem hiding this comment.
pBrData->type is declared as BSP_TEMPLATE_BRANCH_TYPE (see bspeng_co_interface.h), but this assignment casts ucType to BSP_TEMPLATE_OPERATOR instead. This changes the meaning of the persisted/loaded branch data and will break the subsequent switch over branch types. Cast to BSP_TEMPLATE_BRANCH_TYPE (as before) so the enum domain matches the field type.
| pBrData->type = (BSP_TEMPLATE_OPERATOR)ucType; | |
| pBrData->type = (BSP_TEMPLATE_BRANCH_TYPE)ucType; |
| AC_ARG_ENABLE([cord], | ||
| AS_HELP_STRING([--enable-cord],[enable CORD persistent storage support (default is no)]), | ||
| [ | ||
| case "${enableval}" in | ||
| yes) CORD_ENABLED=true | ||
| CFLAGS="$CFLAGS -DCORD_ENABLED";; | ||
| no) CORD_ENABLED=false;; | ||
| *) AC_MSG_ERROR([bad value ${enableval} for --enable-cord]);; | ||
| esac | ||
| ], | ||
| [CORD_ENABLED=false]) | ||
| AM_CONDITIONAL([WITH_CORD_SUPPORT], [test x$CORD_ENABLED = xtrue]) | ||
|
|
There was a problem hiding this comment.
CORD_ENABLED adds new references to cord_* APIs, but the build system here only appends -DCORD_ENABLED to CFLAGS; there’s no library detection/linking (e.g., AC_CHECK_LIB / pkg-config) and WITH_CORD_SUPPORT isn’t referenced elsewhere. As-is, --enable-cord is likely to compile but fail to link (undefined references) or not propagate link flags to the relevant targets. Please add proper dependency checks and wire the conditional into Makefile.am to add the required include/lib flags.
| void FreeList(ERList* pList) | ||
| { | ||
| if(!pList->pHead) return; | ||
| while (pList->pHead) { | ||
| ERListItem *pItem = pList->pHead; | ||
| pList->pHead = pItem->pNext; | ||
| free(pItem); | ||
| } | ||
| } |
There was a problem hiding this comment.
FreeList is defined at file scope but not declared static. This can create a symbol collision at link time if another translation unit defines the same helper name (common for generic helpers). Make it static (or give it a more specific name) since it’s only used within this file under CORD_ENABLED.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 15 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| // Alloc output array | ||
| PCCSP_BASE_RECORD pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); |
There was a problem hiding this comment.
In the CORD-enabled path, ppRecArray is allocated with calloc (and list nodes with malloc), whereas other CCSP APIs in this file allocate output buffers via bus_info->mallocfunc/freefunc. If the project ever uses a custom allocator through bus_info, this can cause inconsistent allocation/free behavior. Consider switching these allocations to the same allocator strategy used elsewhere (or clearly documenting that callers must free() this output).
| // Alloc output array | |
| PCCSP_BASE_RECORD pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); | |
| // Alloc output array using the same allocator strategy as other CCSP APIs. | |
| CCSP_MESSAGE_BUS_INFO *bus_info = (CCSP_MESSAGE_BUS_INFO *)bus_handle; | |
| PCCSP_BASE_RECORD pRecArray = NULL; | |
| size_t recArraySize = list.nCount * sizeof(CCSP_BASE_RECORD); | |
| if (bus_info && bus_info->mallocfunc) { | |
| pRecArray = (PCCSP_BASE_RECORD)bus_info->mallocfunc(recArraySize); | |
| if (pRecArray) { | |
| memset(pRecArray, 0, recArraySize); | |
| } | |
| } else { | |
| pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); | |
| } |
| #ifdef _64BIT_ARCH_SUPPORT_ | ||
| "The child node queue of XML node '%s' is corrupted.cookie2= %p\n", | ||
| #else | ||
| "The child node queue of XML node '%s' is corrupted.cookie2= 0x%.8X\n", | ||
| #endif | ||
| (char *)pXmlNode->AttributesListLock, | ||
| #ifdef _64BIT_ARCH_SUPPORT_ | ||
| (void*)pNameAttr->StringData | ||
| #else | ||
| (unsigned int)pNameAttr->StringData | ||
| #endif |
There was a problem hiding this comment.
Same argument-order issue for the cookie2 warning: the %s placeholder should receive the node name (pNameAttr->StringData), but pXmlNode->AttributesListLock is currently passed as the string. This can lead to invalid memory reads/crashes in the warning path. Please fix the argument order and use a cookie format that matches the cookie’s type.
| //last node name is all integer digits | ||
| len = strlen(pItem->pParameterName); | ||
| if (pItem->pParameterName[len-1] != '.') { | ||
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | ||
| //pRecArray[idxRecArray].InstanceNumber = safe_atou(<last node name, e.g. "123">); | ||
| safe_atou(pItem->pParameterName, &(pRecArray[idxRecArray].Instance.InstanceNumber)); |
There was a problem hiding this comment.
The CORD_TYPE_OBJECT_PATH branch seems to misclassify instances: both objects and instances normally end with '.', and the existing CcspBaseIf_getObjType() logic identifies an instance by the last segment being numeric. With if (pItem->pParameterName[len-1] != '.'), any trailing-dot path will be treated as CCSP_BASE_OBJECT, so instances may never be returned. Please align this detection with CcspBaseIf_getObjType() (numeric last segment => INSTANCE).
| //last node name is all integer digits | |
| len = strlen(pItem->pParameterName); | |
| if (pItem->pParameterName[len-1] != '.') { | |
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | |
| //pRecArray[idxRecArray].InstanceNumber = safe_atou(<last node name, e.g. "123">); | |
| safe_atou(pItem->pParameterName, &(pRecArray[idxRecArray].Instance.InstanceNumber)); | |
| size_t segmentEnd = 0; | |
| size_t segmentStart = 0; | |
| size_t i = 0; | |
| int isInstance = 0; | |
| // Determine whether the last non-empty node name is all integer digits. | |
| len = strlen(pItem->pParameterName); | |
| segmentEnd = len; | |
| while (segmentEnd > 0 && pItem->pParameterName[segmentEnd - 1] == '.') { | |
| segmentEnd--; | |
| } | |
| segmentStart = segmentEnd; | |
| while (segmentStart > 0 && pItem->pParameterName[segmentStart - 1] != '.') { | |
| segmentStart--; | |
| } | |
| if (segmentEnd > segmentStart) { | |
| isInstance = 1; | |
| for (i = segmentStart; i < segmentEnd; i++) { | |
| if (pItem->pParameterName[i] < '0' || pItem->pParameterName[i] > '9') { | |
| isInstance = 0; | |
| break; | |
| } | |
| } | |
| } | |
| if (isInstance) { | |
| char instanceStr[32] = {0}; | |
| size_t instanceLen = segmentEnd - segmentStart; | |
| if (instanceLen >= sizeof(instanceStr)) { | |
| instanceLen = sizeof(instanceStr) - 1; | |
| } | |
| memcpy(instanceStr, pItem->pParameterName + segmentStart, instanceLen); | |
| instanceStr[instanceLen] = 0; | |
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | |
| safe_atou(instanceStr, &(pRecArray[idxRecArray].Instance.InstanceNumber)); |
There was a problem hiding this comment.
While this is true, we can ignore.
If the last segment name isn't numeric we should throw an error while collecting the uints.
We must check safe_atou() return value though.
| #ifdef _64BIT_ARCH_SUPPORT_ | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #else | ||
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | ||
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | ||
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | ||
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | ||
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | ||
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | ||
| #endif |
There was a problem hiding this comment.
The ternary initializers here are relying on a casted boolean being used as the ?: condition (because casts bind tighter than ?:). This is very error-prone and hard to read, and the (ANSC_CRYPTO_KEY*)(ULONG)(...) cast is unrelated to the pointer being assigned. Please restructure so the ?: selects the address first, and only cast the result if needed (ideally no cast at all since both operands are already ANSC_CRYPTO_* pointers).
| #ifdef _64BIT_ARCH_SUPPORT_ | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(ULONG)(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #else | |
| ANSC_CRYPTO_KEY* pMacSecretC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (ANSC_CRYPTO_KEY* )(pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server)? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; | |
| #endif | |
| ANSC_CRYPTO_KEY* pMacSecretC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->MacSecret : &pRecordStateW->MacSecret; | |
| ANSC_CRYPTO_KEY* pMacSecretS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->MacSecret : &pRecordStateR->MacSecret; | |
| ANSC_CRYPTO_KEY* pCipherKeyC = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherKey : &pRecordStateW->CipherKey; | |
| ANSC_CRYPTO_KEY* pCipherKeyS = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherKey : &pRecordStateR->CipherKey; | |
| ANSC_CRYPTO_IV* pCipherIV_C = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateR->CipherIV : &pRecordStateW->CipherIV; | |
| ANSC_CRYPTO_IV* pCipherIV_S = (pSecurityParams->ConnectionEnd == TLS_CONNECTION_END_server) ? &pRecordStateW->CipherIV : &pRecordStateR->CipherIV; |
| //pListItem->pInstanceArray[pList->nCount] = safe_atou(<last node name, e.g. "123">); | ||
| safe_atou(pParameterName, &pList->pInstanceArray[pList->nCount]); | ||
| pList->nCount++; |
There was a problem hiding this comment.
The return value of safe_atou(...) is ignored. If parsing fails, the output element may remain uninitialized (especially after a growth realloc) but nCount is still incremented. Please check the return code and either skip adding the entry or set callbackError and abort enumeration.
There was a problem hiding this comment.
set the callbackError in this case.
| if ( pRecordName[strlen(pRecordName)-1] == '.' ) { | ||
| crc = cord_default_multi(psmName, CORD_FLAG_PERSIST_ASYNC); | ||
| } | ||
| else { | ||
| crc = cord_default(psmName, CORD_FLAG_PERSIST_ASYNC); | ||
| } |
There was a problem hiding this comment.
pRecordName[strlen(pRecordName)-1] will read out of bounds when pRecordName is an empty string. Please guard with a strlen(pRecordName) > 0 check (or treat empty name as invalid) before indexing the last character.
| void FreeList(ERList* pList) | ||
| { | ||
| if(!pList->pHead) return; | ||
| while (pList->pHead) { | ||
| ERListItem *pItem = pList->pHead; | ||
| pList->pHead = pItem->pNext; | ||
| free(pItem); | ||
| } | ||
| } |
There was a problem hiding this comment.
FreeList is declared with external linkage in a C file, which can create duplicate/global symbol collisions at link time. Please make it static (or give it a file-unique name) since it’s only used within this translation unit.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| } | ||
| if (list.callbackError) { | ||
| if (list.pInstanceArray) { | ||
| free(list.pInstanceArray); |
There was a problem hiding this comment.
On callbackError, list.pInstanceArray is released with free(), but in the non-CORD path the instance array is allocated via bus_info->mallocfunc and expected to be freed with bus_info->freefunc. To avoid allocator mismatches, make the CORD path follow the same allocator contract (free with bus_info->freefunc, and allocate with bus_info->mallocfunc).
| free(list.pInstanceArray); | |
| bus_info->freefunc(list.pInstanceArray); |
| PCCSP_BASE_RECORD pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); | ||
| if (!pRecArray) { | ||
| //free linked list | ||
| FreeList(&list); | ||
| return CCSP_Message_Bus_OOM; |
There was a problem hiding this comment.
PsmEnumRecords allocates the returned record array with calloc(), but callers in this codebase commonly free buffers via bus_info->freefunc (see CcspBaseIf_EnumRecords/free_CCSP_BASE_RECORD). If a custom allocator is provided, this can break. Allocate pRecArray with bus_info->mallocfunc (and zero it) to keep the ownership/allocator contract consistent.
| PCCSP_BASE_RECORD pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); | |
| if (!pRecArray) { | |
| //free linked list | |
| FreeList(&list); | |
| return CCSP_Message_Bus_OOM; | |
| PCCSP_BASE_RECORD pRecArray = NULL; | |
| size_t recArraySize = list.nCount * sizeof(CCSP_BASE_RECORD); | |
| if (recArraySize > 0) { | |
| pRecArray = (PCCSP_BASE_RECORD)bus_info->mallocfunc(recArraySize); | |
| if (!pRecArray) { | |
| //free linked list | |
| FreeList(&list); | |
| return CCSP_Message_Bus_OOM; | |
| } | |
| memset(pRecArray, 0, recArraySize); |
| if (!pRecordName) { | ||
| return CCSP_ERR_INVALID_PARAMETER_VALUE; | ||
| } | ||
|
|
||
| if (pSubSystemPrefix && pSubSystemPrefix[0] != 0) { | ||
| rc = strcpy_s(psmName, sizeof(psmName), pSubSystemPrefix); | ||
| ERR_CHK(rc); | ||
| } | ||
| rc = strcat_s(psmName, sizeof(psmName), CCSP_DBUS_PSM); | ||
| ERR_CHK(rc); | ||
| rc = strcat_s(psmName, sizeof(psmName), "."); | ||
| ERR_CHK(rc); | ||
| rc = strcat_s(psmName, sizeof(psmName), pRecordName); | ||
| ERR_CHK(rc); | ||
|
|
||
| if ( pRecordName[strlen(pRecordName)-1] == '.' ) { | ||
| crc = cord_default_multi(psmName, CORD_FLAG_PERSIST_ASYNC); | ||
| } |
| char buf[32]; | ||
| memcpy(buf, start, segLen); | ||
| buf[segLen] = '\0'; | ||
|
|
||
| char *endptr; | ||
| unsigned long val; | ||
| errno = 0; | ||
| val = strtoul(buf, &endptr, 10); |
There was a problem hiding this comment.
The buf[] and memcpy() aren't strictly needed, as strtoul(start, ...) will convert until it hits the . and give you the expected uint value.
| // Alloc output array | ||
| PCCSP_BASE_RECORD pRecArray = (PCCSP_BASE_RECORD)calloc(list.nCount, sizeof(CCSP_BASE_RECORD)); |
| //last node name is all integer digits | ||
| len = strlen(pItem->pParameterName); | ||
| if (pItem->pParameterName[len-1] != '.') { | ||
| pRecArray[idxRecArray].RecordType = CCSP_BASE_INSTANCE; | ||
| //pRecArray[idxRecArray].InstanceNumber = safe_atou(<last node name, e.g. "123">); | ||
| safe_atou(pItem->pParameterName, &(pRecArray[idxRecArray].Instance.InstanceNumber)); |
There was a problem hiding this comment.
While this is true, we can ignore.
If the last segment name isn't numeric we should throw an error while collecting the uints.
We must check safe_atou() return value though.
| /*CID: 110434 Resource leak*/ | ||
| fclose(fp); | ||
| #ifdef CORD_ENABLED | ||
| cord_open(); |
There was a problem hiding this comment.
log the error if this returns failure.
| static int initialized = 0; | ||
| if (initialized) return; | ||
| initialized = 1; |
There was a problem hiding this comment.
Not thread safe - use pthread_once() or at least move the initialized = 1 to end of function or create an intialised table of b64_table[256]={...}, so no code needs to run.
There was a problem hiding this comment.
Done. Initialized the table.
| size_t outbuf_len = 0;; | ||
| unsigned char *outbuf = base64_to_binary(pVal,strlen(pVal),&outbuf_len); | ||
| //Free the pVal, The above fn allocates the buffer for the conversion. | ||
| bus_info->freefunc((void*)pVal); |
There was a problem hiding this comment.
Is the caller expecting us to free the pVal they passed in?
Did you mean to free(outbuf) after cord_set_blob() - as it currently leaks.
There was a problem hiding this comment.
Yes. My intention was to free the outbuf and not pVal. Corrected it.
No description provided.