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

The reproduction of "prov/socket segfaults in sock_ep_connect() when … #606

Closed
wants to merge 1 commit into from

Conversation

dmitrygx
Copy link
Member

@dmitrygx dmitrygx commented Feb 9, 2017

The reproduction of "prov/socket segfaults in sock_ep_connect() when it tries to dereference dest_addr" issue
ofiwg/libfabric#2676

fi_msg_sockets: Add new test case that covers Issue #2676
Invokes fi_send when no connect is established and no destination addres:port
pair is passed to fi_info

Signed-off-by: Dmitry Gladkov [email protected]

…it tries to

dereference dest_addr" issue
ofiwg/libfabric#2676

fi_msg_sockets: Add new test case that covers Issue #2676
Invokes fi_send when no connect is established and no destination addres:port
pair is passed to fi_info

Change-Id: I1a64131eafa882b9f60a725d055ede039ad2250b
Signed-off-by: Gladkov, Dmitry <[email protected]>
@dmitrygx
Copy link
Member Author

Since fix for Issue #2676 isn't included yet onto master branch, the Travis CI build have failed (new added test have failed)

@shefty shefty added this to the release 1.5.0 milestone Feb 10, 2017
if (ret) {
FT_PRINTERR("fi_connect", ret);
free(dest_addr);
Copy link
Member

Choose a reason for hiding this comment

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

This needs the same protection as you have below.

if (ret >= 0) {
FT_ERR("Unexpected result of fi_send - %d (ep %p)", ret, ep);
return -FI_EOTHER;
}
Copy link
Member

Choose a reason for hiding this comment

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

I think this is the only piece of new code that's actually needed. You're just wanting to see what happens if fi_send() is called prior to calling connect. Since this is a coding error in the application, most providers are not going to want the overhead of checking the connection on every send call. So an assertion in the provider is better than a run time failure, which would prevent fabtests from actually trying to catch this error.

@dmitrygx
Copy link
Member Author

Hi @shefty. Since we've added assert instead of "if" statement. I guess, this test isn't applicable anymore and I can close the pull request.

@shefty shefty closed this Feb 13, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants