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

server: Support opening on an existing socket fd #111

Merged
merged 1 commit into from
Jun 2, 2024

Conversation

layercak3
Copy link
Contributor

@layercak3 layercak3 commented May 31, 2024

Tested with a systemd .socket unit in any1/wayvnc#310

Supersedes: #69
Required by: any1/wayvnc#227, any1/wayvnc#310
I have read and understood CONTRIBUTING.md.

Copy link
Owner

@any1 any1 left a comment

Choose a reason for hiding this comment

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

LGTM. Can you squash and force push?

I do wonder if the function should be called nvnc_listen_on_fd or something like that instead of nvnc_open_socket_fd. Other suggestions would be nvnc_open_from_fd or nvnc_create_with_fd. Feel free to pick any of these names or keep it as is.

Add a `struct nvnc* nvnc_open_from_fd(int fd)` function which takes an
existing connection-based socket file descriptor bound by the library
user or a parent process and just calls listen() on it, as an
alternative to letting neatvnc handle socket configuration.
@layercak3
Copy link
Contributor Author

Force-pushed with a new commit calling it nvnc_open_from_fd (twice, since I forgot to stage changes and was using git-diff without arguments instead of comparing HEAD~ HEAD). I'm interpreting open as "open an nvnc handle", not "open a socket", but anyway the "from" should distinguish it from the others while still keeping the same nvnc_open_ prefix used in the public API.

@any1 any1 merged commit 115346f into any1:master Jun 2, 2024
@any1
Copy link
Owner

any1 commented Jun 2, 2024

Thanks!

@layercak3 layercak3 deleted the open-socket-fd branch June 16, 2024 04:47
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