Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Addressing Coverity report: part 1 #1011

Merged
merged 8 commits into from
Nov 25, 2024

Conversation

PanKaker
Copy link
Collaborator

@PanKaker PanKaker commented Nov 22, 2024

Address issues connected with Coverity scan

Copy link
Collaborator

@DawidWesierski4 DawidWesierski4 left a comment

Choose a reason for hiding this comment

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

Hi all of the above are suggestions,

Also i think we should have logging defined as macros

but this is something to consider later in a major sample refactors

app/sample/rdma/rdma_video_rx.c Outdated Show resolved Hide resolved
app/sample/rdma/rdma_video_tx.c Outdated Show resolved Hide resolved
app/sample/rdma/rdma_video_tx_multi.c Outdated Show resolved Hide resolved
@DawidWesierski4
Copy link
Collaborator

Last thing
i belive we should be
1- using inprerative mode in commit messages
This PR addresses some of the issues connected with Coverity scan

could be
Address issues connected with Coverity scan

and the commit message could be
Fix: addressing coverity report

To adhere with the common practices

@Sakoram
Copy link
Collaborator

Sakoram commented Nov 25, 2024

I see some changes in the files related to RDMA. Do we even want to implement RDMA in MTL? Will this feature ever be production ready? I think it was done as an experimental feature, and later we decided that MTL should not be responsible for RDMA.

IMO, all rdma code should be removed. ( The one from Media-Transport-Library/rdma/ AND Media-Transport-Library/lib/src/dev/mt_rdma_ud* )

@PanKaker
Copy link
Collaborator Author

I see some changes in the files related to RDMA. Do we even want to implement RDMA in MTL? Will this feature ever be production ready? I think it was done as an experimental feature, and later we decided that MTL should not be responsible for RDMA.

IMO, all rdma code should be removed. ( The one from Media-Transport-Library/rdma/ AND Media-Transport-Library/lib/src/dev/mt_rdma_ud* )

For now we decided to leave it. We can discuss to clean this up in another PR

Copy link
Collaborator

@DawidWesierski4 DawidWesierski4 left a comment

Choose a reason for hiding this comment

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

LGTM

@PanKaker PanKaker merged commit 296ca24 into OpenVisualCloud:main Nov 25, 2024
24 of 30 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants