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

Allow passing raw fd's into ServerSocket #4156

Merged
merged 5 commits into from
Dec 4, 2023

Conversation

russelltg
Copy link
Contributor

I'm writing an application that receives a fd from systemd that's bound on port 80, and I would like Poco to be able to host the server on the fd that I receive.

This code enables this usecase. I've tested it and it works well

@russelltg russelltg marked this pull request as draft September 22, 2023 00:14
@russelltg
Copy link
Contributor Author

I'm marking this as draft as I realize that it's a breaking change due to the very similar constructor with port.

Maybe instead of a constructor this case should be supported with constructing a default instance then calling a .useFd function or something?

@russelltg russelltg force-pushed the fd_into_serversocket branch from 994e1a9 to 2418861 Compare November 27, 2023 18:45
@russelltg russelltg marked this pull request as ready for review November 27, 2023 18:46
@russelltg
Copy link
Contributor Author

Ok, I think this is a better approach to avoid issues with implicit integer conversion and to make this API use a bit more explicit at callsites.

Thanks for reviewing :)

@aleks-f aleks-f added this to the Release 1.13.0 milestone Nov 27, 2023
Copy link
Member

@aleks-f aleks-f left a comment

Choose a reason for hiding this comment

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

add a test case here, please

@russelltg
Copy link
Contributor Author

I've added a test. I'm not on windows at the moment so haven't tested that the windows portion builds/runs, but it works on Linux.

@russelltg
Copy link
Contributor Author

Okay, test build+passes on Windows now

@matejk
Copy link
Contributor

matejk commented Dec 4, 2023

Code compiles on macOS (clang 15.0.0), unit tests pass.

Copy link
Contributor

@matejk matejk left a comment

Choose a reason for hiding this comment

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

I approve the changes with only one minor comment from my side.

Net/include/Poco/Net/SocketImpl.h Show resolved Hide resolved
@matejk matejk merged commit 3ae282d into pocoproject:devel Dec 4, 2023
3 checks passed
@russelltg russelltg deleted the fd_into_serversocket branch December 4, 2023 15:55
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