internal.c: extract functions to make code cleaner

[SparkiDev]

Description

Extracted to functions pieces of code from very long functions.

Testing

enable-all

[github-actions]

MemBrowse Memory Report

gcc-arm-cortex-m3

• FLASH: .text +32 B (+0.0%, 122,213 B / 262,144 B, total: 47% used)

gcc-arm-cortex-m4-openssl-compat

• FLASH: .rodata +192 B, .text +64 B (+0.0%, 769,748 B / 1,048,576 B, total: 73% used)

gcc-arm-cortex-m4-pq

• FLASH: .text +64 B (+0.0%, 279,048 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m4-rsa-only

• FLASH: .text +64 B (+0.0%, 324,648 B / 1,048,576 B, total: 31% used)

gcc-arm-cortex-m7

• FLASH: .text +64 B (+0.0%, 199,902 B / 262,144 B, total: 76% used)

gcc-arm-cortex-m7-pq

• FLASH: .text +64 B (+0.0%, 279,624 B / 1,048,576 B, total: 27% used)

gcc-arm-cortex-m7-tls13

• FLASH: .text +64 B (+0.0%, 235,792 B / 262,144 B, total: 90% used)

linuxkm-standard

• Data: __patchable_function_entries -8 B (-0.0%, 47,944 B)
  No memory changes detected for:
• gcc-arm-cortex-m0plus
• gcc-arm-cortex-m4
• gcc-arm-cortex-m4-baremetal
• gcc-arm-cortex-m4-crypto-only
• gcc-arm-cortex-m4-dtls13
• gcc-arm-cortex-m4-min-ecc
• gcc-arm-cortex-m4-pkcs7
• gcc-arm-cortex-m4-sp-math
• gcc-arm-cortex-m4-tls12
• gcc-arm-cortex-m4-tls13
• linuxkm-pie
• stm32-sim-stm32h753

[wolfSSL-Fenrir-bot]

Fenrir Automated Review — PR #10841

Scan targets checked: wolfssl-bugs, wolfssl-src

Findings: 1
1 finding(s) posted as inline comments (see file-level comments below)

This review was generated automatically by Fenrir. Findings are non-blocking.

[dgarske]

Skoll Code Review

Scan type: reviewOverall recommendation: COMMENT
Findings: 2 total — 2 posted, 0 skipped
2 finding(s) posted as inline comments (see file-level comments below)

Posted findings

• [Info] New block scopes introduced solely to declare helper-return variables — src/internal.c:7445-7449, 14615-14621, 14704-14708, 14713-14717, 14770-14774
• [Info] skipAddCA now set unconditionally before pending-CA helper call — src/internal.c:17494-17499

Review generated by Skoll

[dgarske]

⚪ [Info] New block scopes introduced solely to declare helper-return variables

The refactor introduces bare { int certRet = ...; } and { int copyRet = ...; } scopes purely to hold a helper's return value. The project C89 style rule states "No new { ... } scope just to declare." That said, this deviation is functionally justified: in SetSSL_CTX a separate certRet is required because assigning the helper's 0-on-success result into the function-level ret would clobber the WOLFSSL_SUCCESS value that must be returned on the writeDup != 0 path; in CopyDecodedToX509 the copyRet pattern preserves error-code priority (only overwrite ret when the new op fails). So the choice is correct, just stylistically at odds with the stated convention.

Fix: Acceptable as-is given the semantics. Optionally hoist the temp declaration to the top of the enclosing function/block to match the C89 convention, but do NOT reuse the function-level ret in SetSSL_CTX (that would change the success return value).

[SparkiDev]

Hoisted the block-scope temps (C89 "no new scope just to declare")

[dgarske]

⚪ [Info] skipAddCA now set unconditionally before pending-CA helper call

In the original inline code, skipAddCA = 1; was set only after ParseCert/AllocDer/MakeSigner/FillSigner all succeeded (just before TLSX_CSR2_AddPendingSigner). The refactor moves skipAddCA = 1; to run unconditionally before ProcessPeerCertAddPendingCA(). This is behaviorally equivalent because skipAddCA is only consulted later under an if (ret == 0 && ... && !skipAddCA) guard (line 17504-17505), and any failure inside the helper returns non-zero and immediately goto exit_ppc, so the differing skipAddCA value on the error path is never read. Flagging only so the intent is documented.

Fix: No change required; behavior is preserved. Optionally add a brief comment noting skipAddCA only matters on the ret==0 continuation path.

[SparkiDev]

Added a comment at the ProcessPeerCertAddPendingCA call site (no behavior change), capturing this.

[dgarske]

This refactor looks great. There is a few empty braces in the refactor. I did a function by function review and the refactor looks good.

[SparkiDev]

Jenkins: retest this please