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

Fix windows fd #25

Merged
merged 6 commits into from Oct 3, 2020
Merged

Fix windows fd #25

merged 6 commits into from Oct 3, 2020

Conversation

sreimers
Copy link
Member

@sreimers sreimers commented Sep 29, 2020

Not ready, needs some rewrites and more tests.

@sreimers sreimers changed the title Fix windows fd Draft: Fix windows fd Sep 29, 2020
@@ -83,15 +83,17 @@ enum {
#endif
};

/** File descriptor handler struct */
struct fhs {
int fd; /**< File Descriptor */
Copy link
Contributor

Choose a reason for hiding this comment

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

int fd is only used on windows ?

perhaps change the type of fd to SOCKET to make it clear that it is a windows type:

typedef UINT_PTR        SOCKET;

it will also let the compiler warn about wrong api usage etc.

Copy link
Member Author

Choose a reason for hiding this comment

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

As i can see SOCKET is casted to int through SOK_CAST like here:
https://github.com/baresip/re/blob/master/src/tcp/tcp.c#L510

I can make re->fhs[i].fd WIN32 only, with some extra ifdefs

@sreimers
Copy link
Member Author

sreimers commented Oct 1, 2020

retest and baresip selftest with wine works fine now (mingw-w64). A simple siege -c10 -t60s 0.0.0.0:8000 performance test with baresip/httpd looks good too (no performance degradation). During the day i will re-check with a real windows 10 and 7 pc.

@sreimers sreimers changed the title Draft: Fix windows fd Fix windows fd Oct 1, 2020
@alfredh
Copy link
Contributor

alfredh commented Oct 3, 2020

I have tested this patch on Debian 10 with the following polling methods:

  • poll
  • select
  • epoll

@sreimers
Copy link
Member Author

sreimers commented Oct 3, 2020

@alfredh thanks for testing. I think its ready to merge. @johnjuuljensen please let me know if it works for you too.

@sreimers sreimers merged commit 7b5aefe into master Oct 3, 2020
@sreimers sreimers deleted the fix_windows_fd branch October 6, 2020 08:33
@johnjuuljensen
Copy link
Contributor

I've tested on windows, and with the pull requests I've submitted on /re and /baresip it works well.

wip-sync pushed a commit to NetBSD/pkgsrc-wip that referenced this pull request Nov 28, 2020
= libre Changelog

== [v1.1.0] - 2020-10-04

=== Added

- tls: functions to get the certificate issuer and subject [#18]
- uri: Added path field to struct uri and its decode to uri_decode [#22]
- tcp: add tcp_connect_bind [#24]
- http: support bind to laddr in http_request [#24]
- sipreg: support Cisco REGISTER keep-alives [#19]
- sip: websocket support [#26]

=== Fixed

- tls/openssl: fix X509_NAME win32/wincrypt.h conflict
- dns: listen on IPv4 and IPv6 socket [#27]
- main: fix/optimize windows file descriptors [#25]

=== Contributors (many thanks)

- Alfred E. Heggestad
- Christian Spielberger
- Christoph Huber
- Franz Auernigg
- Juha Heinanen
- Sebastian Reimers

== [v1.0.0] - 2020-09-08

=== Added

- sip: add trace
- sdp: sdp_media_disabled API function [#2]
- tls: add tls_set_selfsigned_rsa [#6]
- tls: add functions to verify server cert, purpose and hostname [#10]
- http: client should set SNI [#10]
- http: client should use tls functions to verify server certs, purpose
  and hostname [#10]
- sipreg: add proxy expires field and get function [#13]
- sipreg: make re-register interval configurable [#13]

=== Changed

- debian: Automatic cleanup after building debian package

=== Fixed

- Set SDK path (SYSROOT) using xcrun (fix building on macOS 10.14)
- tcp: close socket on windows if connection is aborted or reset [#1]
- rtmp: Fix URL path parsing (creytiv#245)
- ice: various fixes [baresip/baresip#925]
- openssl/tls: replace deprecated openssl 1.1.0 functions [#5]

=== Contributors (many thanks)

- Alfred E. Heggestad
- Christian Spielberger
- Christoph Huber
- Franz Auernigg
- juha-h
- Juha Heinanen
- Richard Aas
- Sebastian Reimers

[#25]: baresip/re#25
[#27]: baresip/re#27
[#26]: baresip/re#26
[#19]: baresip/re#19
[#24]: baresip/re#24
[#22]: baresip/re#22
[#18]: baresip/re#18
[#13]: baresip/re#13
[#10]: baresip/re#10
[#6]: baresip/re#6
[#5]: baresip/re#5
[#2]: baresip/re#2
[#1]: baresip/re#1

[v1.0.0]: baresip/re@v0.6.1...v1.0.0
[v1.1.0]: baresip/re@v1.0.0...v1.1.0
[Unreleased]: baresip/re@v1.1.0...HEAD
@sreimers sreimers added the bug Something isn't working label Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants