Skip to content

Commit 9a70643

Browse files
committed
Unify server handler return code processing - handlers should return
errors to HandleRequestMessage, which should never fail based on these codes
1 parent c907f44 commit 9a70643

File tree

6 files changed

+81
-130
lines changed

6 files changed

+81
-130
lines changed

src/wh_server.c

Lines changed: 30 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -325,6 +325,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
325325
uint16_t seq = 0;
326326
uint16_t size = 0;
327327
uint8_t* data = NULL;
328+
int handlerRc = 0;
328329

329330
if (server == NULL) {
330331
return WH_ERROR_BADARGS;
@@ -342,7 +343,7 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
342343
int rc = wh_CommServer_RecvRequest(server->comm, &magic, &kind, &seq,
343344
&size, data);
344345
/* Got a packet? */
345-
if (rc == 0) {
346+
if (rc == WH_ERROR_OK) {
346347
group = WH_MESSAGE_GROUP(kind);
347348
action = WH_MESSAGE_ACTION(kind);
348349
switch (group) {
@@ -407,33 +408,44 @@ int wh_Server_HandleRequestMessage(whServerContext* server)
407408
#endif /* WOLFHSM_CFG_CERTIFICATE_MANAGER && !WOLFHSM_CFG_NO_CRYPTO */
408409

409410
default:
410-
/* Unknown group. Return empty packet*/
411-
/* TODO: Respond with aux error flag */
411+
/* Unknown group. Return empty packet */
412+
rc = WH_ERROR_NOTIMPL;
412413
size = 0;
413414
}
414415

415-
/* Send a response */
416-
/* TODO: Respond with ErrorResponse if handler returns an error */
416+
/* Capture handler result for logging. The response packet already
417+
* contains the error code for the client in the resp.rc field. */
418+
handlerRc = rc;
419+
420+
/* Handle cancellation by modifying response kind */
417421
#ifdef WOLFHSM_CFG_CANCEL_API
418-
if (rc == WH_ERROR_CANCEL) {
422+
if (handlerRc == WH_ERROR_CANCEL) {
419423
/* notify the client that their request was canceled */
420424
kind = WH_MESSAGE_KIND(WH_MESSAGE_GROUP_CANCEL, 0);
421425
size = 0;
422426
data = NULL;
423-
/* reset RC so the cancellation response is sent */
424-
rc = 0;
425427
}
426428
#endif
427-
if (rc == 0) {
428-
do {
429-
rc = wh_CommServer_SendResponse(server->comm, magic, kind, seq,
430-
size, data);
431-
} while (rc == WH_ERROR_NOTREADY);
432-
}
433-
WH_LOG_ON_ERROR_F(
434-
&server->log, WH_LOG_LEVEL_ERROR, rc,
435-
"Request Handler for (group=%d, action=%d) Returned Error: %d",
436-
group, action, rc);
429+
430+
/* Always send the response to the client, regardless of handler error.
431+
* The response packet contains the operational error code for the
432+
* client in the resp.rc field. */
433+
do {
434+
rc = wh_CommServer_SendResponse(server->comm, magic, kind, seq,
435+
size, data);
436+
} while (rc == WH_ERROR_NOTREADY);
437+
438+
/* Log any communication errors */
439+
WH_LOG_ON_ERROR_F(&server->log, WH_LOG_LEVEL_ERROR, rc,
440+
"SendResponse failed for (group=%d, action=%d): %d",
441+
group, action, rc);
442+
443+
(void)handlerRc; /* Suppress unused variable warning until logging is
444+
* implemented */
445+
446+
/* Always return success when we processed a request, so no handler
447+
* error can terminate the server's request processing loop. */
448+
rc = WH_ERROR_OK;
437449
}
438450

439451
return rc;

src/wh_server_counter.c

Lines changed: 0 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -80,8 +80,6 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
8080
resp.counter = *counter;
8181
}
8282
resp.rc = ret;
83-
/* TODO: are there any fatal server errors? */
84-
ret = WH_ERROR_OK;
8583

8684
(void)wh_MessageCounter_TranslateInitResponse(
8785
magic, &resp, (whMessageCounter_InitResponse*)resp_packet);
@@ -139,9 +137,6 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
139137
magic, &resp, (whMessageCounter_IncrementResponse*)resp_packet);
140138

141139
*out_resp_size = sizeof(resp);
142-
143-
/* TODO: are there any fatal server errors? */
144-
ret = WH_ERROR_OK;
145140
} break;
146141

147142
case WH_COUNTER_READ: {
@@ -178,9 +173,6 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
178173
magic, &resp, (whMessageCounter_ReadResponse*)resp_packet);
179174

180175
*out_resp_size = sizeof(resp);
181-
182-
/* TODO: are there any fatal server errors? */
183-
ret = WH_ERROR_OK;
184176
} break;
185177

186178
case WH_COUNTER_DESTROY: {
@@ -211,9 +203,6 @@ int wh_Server_HandleCounter(whServerContext* server, uint16_t magic,
211203
magic, &resp, (whMessageCounter_DestroyResponse*)resp_packet);
212204

213205
*out_resp_size = sizeof(resp);
214-
215-
/* TODO: are there any fatal server errors? */
216-
ret = WH_ERROR_OK;
217206
} break;
218207

219208
default:

src/wh_server_crypto.c

Lines changed: 1 addition & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -4231,16 +4231,6 @@ int wh_Server_HandleCryptoRequest(whServerContext* ctx, uint16_t magic,
42314231

42324232
WH_DEBUG_SERVER_VERBOSE("End ret:%d\n", ret);
42334233

4234-
/* Since crypto error codes are propagated to the client in the response
4235-
* packet, return success to the caller unless a cancellation has occurred
4236-
*/
4237-
#ifdef WOLFHSM_CFG_CANCEL_API
4238-
if (ret != WH_ERROR_CANCEL) {
4239-
ret = WH_ERROR_OK;
4240-
}
4241-
#else
4242-
ret = WH_ERROR_OK;
4243-
#endif
42444234
return ret;
42454235
}
42464236

@@ -5724,16 +5714,7 @@ int wh_Server_HandleCryptoDmaRequest(whServerContext* ctx, uint16_t magic,
57245714

57255715

57265716
WH_DEBUG_SERVER_VERBOSE("Crypto DMA request. Action:%u\n", action);
5727-
/* Since crypto error codes are propagated to the client in the response
5728-
* packet, return success to the caller unless a cancellation has occurred
5729-
*/
5730-
#ifdef WOLFHSM_CFG_CANCEL_API
5731-
if (ret != WH_ERROR_CANCEL) {
5732-
ret = WH_ERROR_OK;
5733-
}
5734-
#else
5735-
ret = WH_ERROR_OK;
5736-
#endif
5717+
57375718
return ret;
57385719
}
57395720
#endif /* WOLFHSM_CFG_DMA */

0 commit comments

Comments
 (0)