LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device - #15
Conversation
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
Note
Copilot was unable to run its full agentic suite in this review.
This PR updates the message receive/processing loops to exit early when a data encryption step fails, preventing continued processing after an error.
Changes:
- Add
break;after “Data encryption failed” error logging inIDM_Incoming_FT_Response. - Add
break;after “Data encryption failed” error logging inIDM_SFT_receive. - Add inline comments explaining the early-exit behavior.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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 1 comment.
Suppressed comments (1)
source/InterDeviceManager/Idm_msg_process.c:882
- Similar to the earlier receive loop: breaking on bytes<=0 prevents a hang, but the function still returns FT_SUCCESS unconditionally. That can report success even when fewer than total_bytes were received. Return FT_ERROR when length < total_bytes after the loop (and optionally clean up the partial file).
CcspTraceError(("(%s:%d) Socket Read Error (bytes read: %d)\n", __FUNCTION__, __LINE__,bytes));
// Exit from loop if socket is not usable or no data to read
break;
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:509
- When the SSL session is NULL after the output file has been opened, this returns early without deleting the partially created file. Since other error paths now remove partial files, this path should also clean up the destination file.
else{
CcspTraceError(("%s:%d ssl session is null\n",__FUNCTION__,__LINE__));
fclose(fptr);
free(buf);
free(req);
return FT_ERROR;
}
source/InterDeviceManager/Idm_msg_process.c:878
- If the SSL session is NULL after the destination file has been opened, this returns without deleting the partially created file. This can leave empty/partial files on disk despite returning an error.
else{
CcspTraceError(("%s:%d ssl session is null\n",__FUNCTION__,__LINE__));
fclose(fptr);
if(buf){
free(buf);
}
return FT_ERROR;
}
source/InterDeviceManager/Idm_msg_process.c:869
- Spelling in comment: "remianing" should be "remaining".
// read remianing bytes
bytes = SSL_read(conn_info->enc.ssl, buf, total_bytes - length);
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 (4)
source/InterDeviceManager/Idm_msg_process.c:876
- In the SSL-null error path, remove() is called before fclose(). Closing the file first matches the later partial-transfer cleanup and avoids platform/filesystem cases where deletion of an open file can fail.
CcspTraceError(("%s:%d ssl session is null\n",__FUNCTION__,__LINE__));
remove(Data->param_name);
fclose(fptr);
if(buf){
source/InterDeviceManager/Idm_msg_process.c:893
- Same as the earlier function: for SSL builds, this error path would be more actionable if it logged SSL_get_error() instead of only the raw return value.
else{
CcspTraceError(("(%s:%d) Socket Read Error (bytes read: %d)\n", __FUNCTION__, __LINE__,bytes));
// Exit from loop if socket is not usable or broken
break;
source/InterDeviceManager/Idm_msg_process.c:510
- In the SSL-null error path, the code calls remove() while the file is still open. Later cleanup removes the file after fclose(), so reordering here avoids relying on OS-specific semantics and makes the cleanup consistent.
This issue also appears on line 873 of the same file.
CcspTraceError(("%s:%d ssl session is null\n",__FUNCTION__,__LINE__));
remove(req->output_location);
fclose(fptr);
free(buf);
source/InterDeviceManager/Idm_msg_process.c:522
- The new "Socket Read Error" log loses useful diagnostics for SSL reads. Logging SSL_get_error() when SSL_read() returns <= 0 will make failures actionable during debugging.
This issue also appears on line 890 of the same file.
else{
CcspTraceError(("(%s:%d) Socket Read Error (bytes read: %d)\n", __FUNCTION__, __LINE__,bytes));
// Exit from loop if socket is not usable or broken
break;
Reason for change: Exit from loop if socket is not usable
Test Procedure: NA
Risks: Low
Signed-off-by: biju.vijayanindiradevi@sky.uk