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

Added socket timeout support #54

Merged
merged 2 commits into from
May 2, 2022
Merged

Added socket timeout support #54

merged 2 commits into from
May 2, 2022

Conversation

gloveboxes
Copy link
Contributor

No description provided.

@gloveboxes
Copy link
Contributor Author

gloveboxes commented May 2, 2022

@Theldus PR done, I updated the examples too. But I see that it doesn't build on (via Wine) :(

As a matter of interest, why don't you use WSL on Windows?

dg

@Theldus
Copy link
Owner

Theldus commented May 2, 2022

@gloveboxes Thanks, since this PR contains changes on top of my PR (#53), I'll merge it and then yours, ok?

But I see that it doesn't build on (via Wine) :(

Looking at Travis CI logs, this happened because you added '#include <sys/socket.h>' and '#include <sys/time.h>' in ws.h, and Windows doesn't have the '<sys/socket.h>' header. In fact, I believe you don't need these headers in ws.h, as ws.c already includes them for you, depending on the platform.

As a matter of interest, why don't you use WSL on Windows?

To be honest I've never used it, whenever I needed to compile something on Windows I used Cygwin or MinGW, so it was natural for me. I also believe that it is easier to use Cross-Compiler + Wine in TravisCI than to configure a Windows environment.

From a practical point of view, WSL appears to generate Linux binaries in Windows environments while MinGW generates native binaries for Windows and without any external dependencies.

So a project using wsServer + MinGW is able to run on various versions of Windows (Windows 7 included) without requiring anything other than the .dll's that Windows already provides.

@Theldus Theldus merged commit b358859 into Theldus:pingpong_v2 May 2, 2022
@Theldus
Copy link
Owner

Theldus commented May 2, 2022

@gloveboxes Sorry, I just saw that you sent the PR to 'pingpong_v2' instead of master.

But I've added your changes in master, and fixed the build problem too =).
Thanks a lot for the changes, Ping/Pong should work as expected now.

@gloveboxes
Copy link
Contributor Author

@Theldus perfect - thank you for fixing and thanks again for the project. I have another suggestion/optimization, it is very modest, will open an issue. Thanks, Dave

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.

2 participants