Skip to content

Echoserver Channel Regrouping#962

Merged
padelsbach merged 1 commit intowolfSSL:masterfrom
ejohnstown:echoserver-channels
May 5, 2026
Merged

Echoserver Channel Regrouping#962
padelsbach merged 1 commit intowolfSSL:masterfrom
ejohnstown:echoserver-channels

Conversation

@ejohnstown
Copy link
Copy Markdown
Contributor

Refactor of the echoserver example only; library is unchanged.

  1. Collapse FwdStates and AGENT_STATE_* into a single WS_AppState (INIT/LISTEN/CONNECT/CONNECTED). FWD_STATE_DIRECT becomes an isDirect flag, cleared after connect.
  2. Hoist the fields shared by the forwarding, agent, and shell paths into a common WS_AppCtx (privateData, listenFd, appFd, channelId, state, buffer). Shell only uses buffer.
  3. Feature-specific remainder stays in WS_AgentCbActionCtx (name) and WS_FwdCbActionCtx (host/origin name+port, isDirect). Drop the unused pid field; inline getpid() at the call site.
  4. wolfSSH_AGENT_DefaultActions and wolfSSH_FwdDefaultActions now take a WS_AppCtx* and reach feature-specific state via appCtx->privateData. echoserver_test() is updated to register the WS_AppCtx (not the old CbCtx) with wolfSSH_set_agent_cb_ctx and wolfSSH_SetFwdCbCtx.
  5. Reindent ssh_worker()'s #ifdef blocks and add /* WOLFSSH_xxx */ markers.
  6. fwd.test.expect: redirect echoserver and portfwd output to log files and sync on portfwd's -R ready file instead of matching "sampled" in the pty. Under --enable-debug the WLOG volume fills the pty and stalls portfwd in select() before channel-open.

Copilot AI review requested due to automatic review settings May 4, 2026 22:11
Copy link
Copy Markdown
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 refactors the echoserver example’s channel/forwarding/agent state handling by consolidating per-feature state into a shared WS_AppCtx (with a unified WS_AppState), and updates the forwarding expect test to avoid deadlocks caused by verbose debug logging filling the PTY buffer.

Changes:

  • Consolidate forwarding and agent state machines into WS_AppState, and hoist shared fields into WS_AppCtx with per-feature “private” state hung off privateData.
  • Update agent/fwd default action callbacks to accept a WS_AppCtx* and fetch feature-specific state via appCtx->privateData.
  • Make scripts/fwd.test.expect more robust under --enable-debug by redirecting output to files and syncing on portfwd’s -R ready file.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 4 comments.

File Description
scripts/fwd.test.expect Redirects server/client output to log files and waits on a ready file instead of PTY output matching to prevent debug-log PTY stalls.
examples/echoserver/echoserver.c Refactors example forwarding/agent/shell state into WS_AppCtx/WS_AppState and updates callbacks + worker loop to use the regrouped state.

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

Comment thread examples/echoserver/echoserver.c
Comment thread examples/echoserver/echoserver.c
Comment thread examples/echoserver/echoserver.c
Comment thread examples/echoserver/echoserver.c
@ejohnstown ejohnstown force-pushed the echoserver-channels branch from 187a347 to b0a10a2 Compare May 4, 2026 22:36
Refactor of the echoserver example only; library is unchanged.

1. Collapse FwdStates and AGENT_STATE_* into a single WS_AppState
   (INIT/LISTEN/CONNECT/CONNECTED). FWD_STATE_DIRECT becomes an
   isDirect flag, cleared after connect.
2. Hoist the fields shared by the forwarding, agent, and shell paths
   into a common WS_AppCtx (privateData, listenFd, appFd, channelId,
   state, buffer). Shell only uses buffer.
3. Feature-specific remainder stays in WS_AgentCbActionCtx (name) and
   WS_FwdCbActionCtx (host/origin name+port, isDirect). Drop the unused
   pid field; inline getpid() at the call site.
4. wolfSSH_AGENT_DefaultActions and wolfSSH_FwdDefaultActions now take
   a WS_AppCtx* and reach feature-specific state via
   appCtx->privateData. echoserver_test() is updated to register the
   WS_AppCtx (not the old CbCtx) with wolfSSH_set_agent_cb_ctx and
   wolfSSH_SetFwdCbCtx.
5. Reindent ssh_worker()'s #ifdef blocks and add /* WOLFSSH_xxx */
   markers.
6. fwd.test.expect: redirect echoserver and portfwd output to log files
   and sync on portfwd's -R ready file instead of matching "sampled" in
   the pty. Under --enable-debug the WLOG volume fills the pty and
   stalls portfwd in select() before channel-open.
Copy link
Copy Markdown
Contributor

@padelsbach padelsbach left a comment

Choose a reason for hiding this comment

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

A couple minor suggestions, but looks good overall

Comment thread examples/echoserver/echoserver.c
Comment thread scripts/fwd.test.expect
@padelsbach padelsbach merged commit 834e60f into wolfSSL:master May 5, 2026
131 checks passed
@ejohnstown ejohnstown deleted the echoserver-channels branch May 5, 2026 16:05
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.

5 participants