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

Update TCP proxy to address Coverity errors #3422

Merged
merged 1 commit into from
Feb 4, 2025

Conversation

matt335672
Copy link
Member

This is a follow-on from #3421 which appears to have triggered another warning.

Coverity seems to have some problems with the loop(s) copying data from one socket to another, in that it assume that eventually an integer overflow will occur. It's not obvious why this should be flagged, but this seems likely to be a false positive. There are guards around the integer operations in question.

This commit avoids the integer issue by using a simple pointer + count mechanism to copy the data.

The socket copy code has been placed in a separate function - before it was duplicated. Minor fixes have been made to error reporting around the connection code.

Coverity seems to have some problems with the loop(s) copying data from
one socket to another, in that it assume that eventually an integer
overflow will occur. It's not obvious why this should be flagged, but
this seems likely to be a false positive.

This commit avoids the integer issue by using a simple pointer + count
mechanism.

The socket copy code has been placed in a separate function - before it
was duplicated. Minor fixes have been made to error reporting around the
connection code.
@matt335672 matt335672 merged commit aae69f4 into neutrinolabs:devel Feb 4, 2025
14 checks passed
@matt335672 matt335672 deleted the coverity_scan branch February 4, 2025 12:31
@matt335672
Copy link
Member Author

Merged to pick up the overnight Coverity scan

At some stage I'll batch up #3421 and #3422 and possibly others for a backport to v0.10

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.

1 participant