LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device - #14
LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device#14biju-vi wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Pull request overview
Addresses a hang during file-transfer receive processing by exiting a tight read loop when the socket becomes unusable, preventing IDM from getting stuck and impacting remote-device discovery.
Changes:
- Added loop-exit behavior when socket reads fail / return no data in two receive paths.
- Updated receive-loop control flow to avoid spinning indefinitely.
Suppressed comments (1)
source/InterDeviceManager/Idm_msg_process.c:883
- When
bytes <= 0this breaks out of the loop but the function later returnsFT_SUCCESS, which can silently treat a partial/failed file receive as successful. Also, the read size usestotal_bytes-bytes(last read size) instead of the remaining bytes (total_bytes - length), which can request more data than remains and write past the intended file size. Usetotal_bytes - lengthand return an error after cleanup onbytes <= 0.
else{
CcspTraceError(("(%s:%d) Data encryption failed (Err: %d)\n", __FUNCTION__, __LINE__,bytes));
//exit from tight loop if read socket is not usable or there is no data to read
break;
}
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
source/InterDeviceManager/Idm_msg_process.c:883
- Using
breakhere will still result inFT_SUCCESSbeing returned later, which can incorrectly report a successful receive even when the socket read failed or the peer closed the connection. This should return an error (after cleanup) rather than exiting the loop and continuing.
else{
CcspTraceError(("(%s:%d) Data encryption failed (Err: %d)\n", __FUNCTION__, __LINE__,bytes));
//exit from tight loop if read socket is not usable or there is no data to read
break;
}
source/InterDeviceManager/Idm_msg_process.c:523
- This branch currently
breaks out of the receive loop, but the function will still proceed toreturn FT_SUCCESSafterward. That can report a successful transfer while only a partial file was received (truncated file on disk). This should treatbytes <= 0as a receive failure and return an error after cleanup; also the log message refers to encryption even though this is a socket/SSL read failure.
CcspTraceError(("(%s:%d) Data encryption failed (Err: %d)\n", __FUNCTION__, __LINE__,bytes));
// Exit from tight loop if read socket is not usable or there is no data to read
break;
}
}
RDKB-63310 - Fix IDM Coverity report issues LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device Reason for change: Exit from loop if socket is not usable Test Procedure: NA Risks: Low Signed-off-by: biju.vijayanindiradevi@sky.uk LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device Reason for change: Exit from loop if socket is not usable Test Procedure: NA Risks: Low Signed-off-by: biju.vijayanindiradevi@sky.uk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
source/InterDeviceManager/Idm_msg_process.c:539
- IDM_Incoming_FT_Response can return FT_SUCCESS after breaking out of the receive loop early (e.g., socket error) as long as some bytes were read. That treats partial/incomplete transfers as success and can leave a truncated file on disk.
if(length > 0 )
{
return FT_SUCCESS;
}
return FT_ERROR;
source/InterDeviceManager/Idm_msg_process.c:904
- IDM_SFT_receive can break out of the receive loop on read/SSL errors but still returns FT_SUCCESS as long as at least one byte was written. This marks partial/incomplete transfers as success and can leave a truncated file.
if(length > 0 )
{
return FT_SUCCESS;
}
return FT_ERROR;
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (2)
source/InterDeviceManager/Idm_msg_process.c:539
- The function can return FT_SUCCESS even when the file transfer ended early (e.g., after breaking out of the read loop due to socket/SSL read failure). This risks treating a partially written file as a successful transfer. Returning success should require that the expected number of bytes were received (length == total_bytes), and should not succeed when payload is NULL.
if(length > 0 )
{
return FT_SUCCESS;
}
return FT_ERROR;
source/InterDeviceManager/Idm_msg_process.c:904
- The updated return condition treats any non-zero received bytes as FT_SUCCESS, even if the loop exited early due to a read error (break). This can report success for partial transfers. Success should be returned only when the full expected size was received (length == total_bytes).
if(length > 0 )
{
return FT_SUCCESS;
}
return FT_ERROR;
Reason for change: Exit from loop if socket is not usable Test Procedure: NA Risks: Low Signed-off-by: biju.vijayanindiradevi@sky.uk
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.
Suppressed comments (3)
source/InterDeviceManager/Idm_msg_process.c:522
- On read/SSL_read failure the loop
breaks and the function later returnsFT_ERROR, but the partially written output file is left on disk. This can leave a corrupt file atreq->output_locationfor callers that don't strictly gate on the return value.
else{
CcspTraceError(("(%s:%d) Data encryption failed (Err: %d)\n", __FUNCTION__, __LINE__,bytes));
// Exit from tight loop if read socket is not usable or there is no data to read
break;
}
source/InterDeviceManager/Idm_msg_process.c:886
- On read/SSL_read failure the loop
breaks and the function later returnsFT_ERROR, but the partially written destination file remains on disk atData->param_name. Consider deleting the file when the transfer does not complete to avoid leaving a corrupt artifact.
else{
CcspTraceError(("(%s:%d) Data encryption failed (Err: %d)\n", __FUNCTION__, __LINE__,bytes));
//Exit from tight loop if read socket is not usable or there is no data to read
break;
}
source/InterDeviceManager/Idm_msg_process.c:539
- When
payloadis NULL,lengthandtotal_bytesremain 0, so the finalif (length == total_bytes)returnsFT_SUCCESS. This incorrectly reports success for a NULL payload (and could mask other early-exit paths that leavetotal_bytesunchanged).
else
{
CcspTraceError(("%s:%d payload is null\n",__FUNCTION__, __LINE__));
}
if(length == total_bytes )
{
return FT_SUCCESS;
}
return FT_ERROR;
Reason for change: Exit from loop if socket is not usable Test Procedure: NA Risks: Low Signed-off-by: biju.vijayanindiradevi@sky.uk
Reason for change: Exit from loop if socket is not usable Test Procedure: NA Risks: Low Signed-off-by: biju.vijayanindiradevi@sky.uk
| if(length < total_bytes) | ||
| { | ||
| // remove partially filled file | ||
| remove(req->output_location) |
Reason for change: Exit from loop if socket is not usable
Test Procedure: NA
Risks: Low
Signed-off-by: biju.vijayanindiradevi@sky.uk