System Configuration
The following is my configuration.
- uc/USB: CDC-ACM
- uc/USB: CDC-EEM
- uc/Shell: Shell
- uc/Shell: Terminal
- uc/TCPIP
The terminal is accessed through a serial port which is hosted over the CDC-ACM port. The shell at startup registered all net commands.
Bug Overview
NetICMP_TxEchoReq() function can return the wrong error code. In the following code, if kal_err != KAL_ERR_NONE
then an error has occurred. For any error after setting p_err
the software will issue a jump to release
.
switch (kal_err) {
case KAL_ERR_NONE:
result = DEF_OK;
*p_err = NET_ICMP_ERR_NONE;
goto release;
case KAL_ERR_TIMEOUT:
result = DEF_FAIL;
*p_err = NET_ICMP_ERR_ECHO_REQ_TIMEOUT;
goto release;
case KAL_ERR_ABORT:
case KAL_ERR_ISR:
case KAL_ERR_WOULD_BLOCK:
case KAL_ERR_OS:
default:
result = DEF_FAIL;
*p_err = NET_ICMP_ERR_ECHO_REQ_SIGNAL_FAULT;
goto release;
}
release:
NetICMP_LockAcquire(p_err);
KAL_SemDel(sem_handle, &kal_err);
if (*p_err == NET_ICMP_ERR_NONE) {
if (data_len > 0) {
if (p_echo_req_handle_new->DataCmp != DEF_OK) {
*p_err = NET_ICMP_ERR_ECHO_REPLY_DATA_CMP_FAIL;
}
}
}
Once execution resumes at release
, the first function, NetICMP_LockAcquire(p_err)
, will overwrite p_err
resulting in any previous error code being lost.
Reproduction
- Start system. System not connected to internet.
- Connect to system over serial port (using USB CDC-ACM).
- Issue a net_ping command using IP address that is not on local network.
- Shell prints error error code 12033 (
NET_ICMP_ERR_ECHO_REPLY_DATA_CMP_FAIL
).
Shell should return NET_ICMP_ERR_ECHO_REQ_TIMEOUT
.
Debugging shows that p_err
does get set correctly to NET_ICMP_ERR_ECHO_REQ_TIMEOUT
, but is overwritten once code following release
starts executing.
Resolution
If the switch statement is changes so that errors use goto exit_kal_fault
instead of goto release
, the system will not call Mem_DynPoolBlkFree()
which results in a memory leak. Since this code uses goto, the following code section
exit_kal_fault:
Mem_DynPoolBlkFree(&NetICMP_DataPtr->EchoReqPool,
p_echo_req_handle_new,
&lib_err);
exit_release_lock:
NetICMP_LockRelease();
exit_return:
return (result);
could be replaced with
exit_kal_fault:
Mem_DynPoolBlkFree(&NetICMP_DataPtr->EchoReqPool,
p_echo_req_handle_new,
&lib_err);
exit_release_lock:
NetICMP_LockRelease();
goto exit_return;
exit_kal_fault_with_cleanup:
Mem_DynPoolBlkFree(&NetICMP_DataPtr->EchoReqPool,
p_echo_req_handle_new,
&lib_err);
exit_return:
return (result);
where exit_kal_fault_with_cleanup
will be used in the switch case instead of goto release
for the fault conditions.
As I can't find a coding standard I do not know what the appropriate solution would be.