Skip to content

Conversation

anebot
Copy link

@anebot anebot commented Jan 30, 2019

I've just recap that once you call Disconnect() from your socket ptr is not set again to NULL, so a subsequent call to connected() it will return true regardless the socket is really disconnected.

@sigiesec
Copy link
Member

Thanks for the PR. I fear this change ia not valid. The behaviour of connect/disconnect is different: You can connect to multiple endpoints, disconnect from some and connect again, for example. Apparently there are no tests for this.

And you're right, connected is names misleadingly. A better name would be valid or opened, since calling close will make a socket invalid.

@coveralls
Copy link

coveralls commented Jan 30, 2019

Pull Request Test Coverage Report for Build 159

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 4 unchanged lines in 1 file lost coverage.
  • Overall coverage decreased (-0.1%) to 45.591%

Files with Coverage Reduction New Missed Lines %
zmq.hpp 4 68.79%
Totals Coverage Status
Change from base Build 157: -0.1%
Covered Lines: 1918
Relevant Lines: 4207

💛 - Coveralls

@anebot
Copy link
Author

anebot commented Jan 30, 2019

Thanks for the PR. I fear this change ia not valid. The behaviour of connect/disconnect is different: You can connect to multiple endpoints, disconnect from some and connect again, for example. Apparently there are no tests for this.

And you're right, connected is names misleadingly. A better name would be valid or opened, since calling close will make a socket invalid.

Sure. I've misunderstood the function. As you already pointed function's name might be misleading.

Another possibility, to be coherent with the function's name could be adding a parameter in order to check if the socket is connected to an endpoint.
But I understand that keep the list of connected endpoints can be tedious.

@sigiesec
Copy link
Member

The underlying libzmq doesn't allow to query the list of connected endpoints, so this would need to be done on the cppzmq, which is not quite in line with the "thin layer" approach of cppzmq. Also, be aware that "connected" in the terms of libzmq doesn't mean there is an existing network (e.g. TCP) connection, just that it could connect if the peer is available.

@sigiesec
Copy link
Member

sigiesec commented Feb 4, 2019

@anebot I am closing this PR, since all changes are reverted.

@sigiesec sigiesec closed this Feb 4, 2019
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.

3 participants