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.

Sign up for GitHub

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

Jump to bottom

A clean room re-implementation of zmq_proxy_steerable() #4598

Merged
merged 1 commit into from
Oct 9, 2023
Merged

A clean room re-implementation of zmq_proxy_steerable() #4598

merged 1 commit into from
Oct 9, 2023

Conversation

brettviren
Copy link
Contributor

Here is my attempt to provide #4597

This is my first real dive into the internals of libzmq so there may be some problems that need to be ironed out.

@bluca
Copy link
Member

bluca commented Oct 8, 2023 

../../src/proxy.cpp:231:27: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    stats_proxy stats = { 0 };
                          ^
                          {}
../../src/proxy.cpp:231:27: error: suggest braces around initialization of subobject [-Werror,-Wmissing-braces]
    stats_proxy stats = { 0 };
                          ^
                          {}

@bluca
Copy link
Member

bluca commented Oct 8, 2023

Please state in the commit message that this is a clean room implementation with no knowledge of the previous implementation

@bluca
Copy link
Member

bluca commented Oct 8, 2023 

C:\projects\libzmq\src\proxy.cpp(363,52): error C2220: the following warning is treated as an error [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(363,52): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(369,50): warning C4267: 'argument': conversion from 'size_t' to 'int', possible loss of data [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(376,26): error C2146: syntax error: missing ')' before identifier 'and' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(376,26): error C2065: 'and': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(376,30): error C2146: syntax error: missing ';' before identifier 'events' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(376,58): error C2059: syntax error: ')' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(377,60): error C2059: syntax error: ';' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp([382](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L382),24): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp([383](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L383),39): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
  pull.cpp
C:\projects\libzmq\src\proxy.cpp([384](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L384),40): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp([388](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L388),28): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp([389](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L389),42): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
  push.cpp
C:\projects\libzmq\src\proxy.cpp([390](https://ci.appveyor.com/project/zeromq/libzmq/builds/48221999/job/k2t27ri95ylbajmj#L390),43): error C2065: 'i': undeclared identifier [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(473,5): error C2059: syntax error: 'do' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(473,5): error C2143: syntax error: missing ';' before '{' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(473,5): error C2447: '{': missing function header (old-style formal list?) [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(473,5): error C2059: syntax error: 'while' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(474,5): error C2059: syntax error: 'return' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(475,1): error C2059: syntax error: '}' [C:\projects\build_libzmq\libzmq.vcxproj]
C:\projects\libzmq\src\proxy.cpp(475,1): error C2143: syntax error: missing ';' before '}' [C:\projects\build_libzmq\libzmq.vcxproj]

@brettviren
Copy link
Contributor Author

Thanks, @bluca. I'm working on this now.

@brettviren brettviren force-pushed the reimplement-proxy-steerable branch from 6994761 to 1e94dd4 Compare October 8, 2023 17:38
@brettviren
Copy link
Contributor Author

@bluca as far as I can understand the failed/cancelled tests are not related to these changes.

Please let me know if you spot other improvements that are needed.

@bluca
Copy link
Member

bluca commented Oct 8, 2023

please rebase and squash the fixups

@bluca
Copy link
Member

bluca commented Oct 8, 2023

this also needs to be fixed https://github.com/zeromq/libzmq/actions/runs/6449567065/job/17508023155?pr=4598

@brettviren brettviren force-pushed the reimplement-proxy-steerable branch from 4ce7d70 to 26f5b11 Compare October 8, 2023 20:38
@brettviren
Copy link
Contributor Author

@bluca clang formatted and commit squashed. Is the failed test or the two that timed out pointing to something still broken?

bluca
bluca reviewed Oct 8, 2023
View reviewed changes
src/proxy.cpp Outdated Show resolved Hide resolved
@brettviren
A clean-room implementation of zmq_proxy_steerable().
5712ad5 
It is contriubted under the MPL-2.0.

I had no knowledge of the previous implementation of zmq_proxy_steerable().

This version was developed based on expectations set in the old man page with one exception.  This version uses a REP/REQ for the proxy control protocol sockets.  The old man page example used PUB/SUB which is nonsensical given the STATISTICS command requires two way communication.
@brettviren brettviren force-pushed the reimplement-proxy-steerable branch from 26f5b11 to 5712ad5 Compare October 8, 2023 23:56
@bluca bluca merged commit 5cf39bf into zeromq:master Oct 9, 2023
15 of 19 checks passed
@bluca bluca linked an issue Oct 9, 2023 that may be closed by this pull request
zmq_proxy_steerable needs properly licensed re-implementation #4597
Closed
@nnog nnog mentioned this pull request Oct 12, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bluca bluca bluca left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

zmq_proxy_steerable needs properly licensed re-implementation
3 participants
@brettviren @bluca @BYVoid