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

LuaSocket: Bump to 20240524 #1827

Merged
merged 1 commit into from
Jun 18, 2024
Merged

LuaSocket: Bump to 20240524 #1827

merged 1 commit into from
Jun 18, 2024

Conversation

NiLuJe
Copy link
Member

@NiLuJe NiLuJe commented Jun 17, 2024

And enforce CLOEXEC on open'ed sockets, to avoid weird crap like koreader/koreader#12043


This change is Reviewable

And enforce CLOEXEC on open'ed sockets, to avoid weird crap like koreader/koreader#12043
@Frenzie
Copy link
Member

Frenzie commented Jun 17, 2024

Is it suitable to PR upstream as is?

@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 17, 2024

That's a fair question. I have no idea if there's a Windows equivalent, and/or if it's something upstream would even want. (I imagine there are use-cases where you want your child processes to inherit sockets).

@benoit-pierre
Copy link
Contributor

Wouldn't using SOCK_CLOEXEC make more sense?

SOCK_CLOEXEC Set the close-on-exec (FD_CLOEXEC) flag on the new file descriptor. See the description of the O_CLOEXEC flag in open(2) for reasons why this may be useful.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 17, 2024

It would, but it might not be supported by the crappy old kernels found on our oldest devices (and it might actually make the socket call fail instead of silently ignoring the unknown flag, I can't recall).

So, using fcntl is, unfortunately, a portability necessity.

@NiLuJe
Copy link
Member Author

NiLuJe commented Jun 17, 2024

If I check my notes from the last time I looked into it, as far as Kobo is concerned, it would be safe, the only CLOEXEC compat issue I'm aware of is the lack of support for accept4; open/socket/inotify are OK.

I know that open is NOT okay on legacy Kindle/PB, though, so I'm assuming that's also true of socket there ;).

@NiLuJe NiLuJe merged commit 7b14647 into koreader:master Jun 18, 2024
3 checks passed
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.

None yet

3 participants