Skip to content

Conversation

@bigbrett
Copy link
Contributor

@bigbrett bigbrett commented Dec 12, 2025

Further integration of the new logging framework, with some associated refactors and a new back end.

  • Refactors server request handling to enforce a more uniform structure:
    • Server handlers always populate the response sent to the client with the appropriate data/return code, then return their own error codes to the caller (wh_Server_HandleRequestMessage).
    • wh_Server_HandleRequestMessage now observes handler error codes and acts as a centralized point for logging failures. A response is always sent to the client, and no error is ever propagated to the caller, ensuring handler errors cannot terminate the server.
  • Adds centralized logging for handler failures to server request message processing, based on the above
  • Adds a simple printf logging backend for ephemeral console-based logs

errors to HandleRequestMessage, which should never fail based on these
codes
@bigbrett bigbrett force-pushed the logging-invocations branch from 55062fa to ade0b5a Compare December 12, 2025 22:28
@bigbrett bigbrett force-pushed the logging-invocations branch from ade0b5a to 7d2d372 Compare December 12, 2025 22:28
@bigbrett bigbrett changed the title Logging part 2 Logging: part 2 Dec 12, 2025
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR introduces the second phase of the logging framework integration, refactoring server request handlers to enforce uniform error handling and adding a new printf-based logging backend for console output.

Key Changes:

  • Refactors all server request handlers to return actual error codes (instead of unconditionally returning OK) which are then logged centrally in wh_Server_HandleRequestMessage, preventing handler failures from terminating the server
  • Adds a new wh_log_printf backend providing ephemeral console-based logging
  • Introduces wh_Log_LevelToString() utility function to eliminate code duplication across logging backends

Reviewed changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
wolfhsm/wh_log_printf.h New header defining the printf logging backend interface with context and configuration structures
src/wh_log_printf.c Implementation of printf logging backend with conditional logging based on debug configuration
wolfhsm/wh_log.h Adds declaration for new wh_Log_LevelToString() utility function
src/wh_log.c Implements wh_Log_LevelToString() to convert log level enums to string representations
port/posix/posix_log_file.c Refactored to use shared wh_Log_LevelToString() instead of local implementation
wolfhsm/wh_error.h Updates comment for WH_ERROR_NOHANDLER to clarify its general purpose
src/wh_server.c Centralizes request handler error logging and ensures handlers never terminate the server
src/wh_server_keystore.c Removes conditional error suppression logic; handlers now return actual error codes for centralized logging
src/wh_server_crypto.c Removes error suppression logic that unconditionally returned OK
src/wh_server_counter.c Removes TODOs and error suppression comments
src/wh_server_she.c Returns actual error codes instead of unconditionally returning success
Comments suppressed due to low confidence (1)

src/wh_server_keystore.c:1564

  • When ret != WH_ERROR_OK, resp.len and resp.label are not set, leaving them uninitialized. The response structure should either be initialized with = {0} on line 1542, or these fields should be explicitly zeroed in the error path to avoid sending uninitialized data to the client.
            if (ret == WH_ERROR_OK) {
                resp.len = req.key.sz;
                memcpy(resp.label, meta->label, sizeof(meta->label));
            }

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@bigbrett bigbrett force-pushed the logging-invocations branch from 7d2d372 to 3fba076 Compare December 12, 2025 22:49
@bigbrett bigbrett marked this pull request as ready for review December 12, 2025 22:50
Copy link
Contributor

@billphipps billphipps 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! Had a single concern about always squashing error code within server handler.

@bigbrett bigbrett requested a review from billphipps December 18, 2025 00:06
Copy link
Contributor

@dgarske dgarske left a comment

Choose a reason for hiding this comment

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

Very nice PR! Tests out with customer ST SR6 hardware. Thank you Brett

@dgarske dgarske merged commit 1e47a7e into wolfSSL:main Dec 18, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants