Skip to content

LTE-3039: IDM is stuck in XLE causing XB to not discover Remote Device - #15

Merged
guto86 merged 4 commits into
rdkcentral:mainfrom
biju-vi:socket
Aug 18, 2026
Merged

guto86 merged 4 commits into
rdkcentral:mainfrom
biju-vi:socket

Conversation

@biju-vi

@biju-vi biju-vi commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

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
Copilot AI review requested due to automatic review settings August 6, 2026 16:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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 in IDM_Incoming_FT_Response.
  • Add break; after “Data encryption failed” error logging in IDM_SFT_receive.
  • Add inline comments explaining the early-exit behavior.

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

Comment thread source/InterDeviceManager/Idm_msg_process.c Outdated
Comment thread source/InterDeviceManager/Idm_msg_process.c Outdated
Comment thread source/InterDeviceManager/Idm_msg_process.c Outdated
@biju-vi
biju-vi requested a review from Copilot August 6, 2026 16:32

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@biju-vi
biju-vi requested a review from Copilot August 6, 2026 17:46

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Reason for change: Exit from loop if socket is not usable

Test Procedure: NA

Risks: Low
Signed-off-by: biju.vijayanindiradevi@sky.uk
Copilot AI review requested due to automatic review settings August 6, 2026 18:25

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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;

Comment thread source/InterDeviceManager/Idm_msg_process.c
Reason for change: Exit from loop if socket is not usable

Test Procedure: NA

Risks: Low
Signed-off-by: biju.vijayanindiradevi@sky.uk
Copilot AI review requested due to automatic review settings August 7, 2026 09:16

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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
Copilot AI review requested due to automatic review settings August 7, 2026 10:09

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Pull request overview

Copilot reviewed 1 out of 1 changed files in this pull request and generated no new comments.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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;

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot encountered an error and was unable to review this pull request. You can try again by re-requesting a review.

@guto86
guto86 merged commit 36b992e into rdkcentral:main Aug 18, 2026
3 checks passed
@github-actions github-actions Bot locked and limited conversation to collaborators Aug 18, 2026
@biju-vi
biju-vi deleted the socket branch August 18, 2026 11:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants