Skip to content

unix_fd_add: Port to safe-io#1745

Open
A6GibKm wants to merge 1 commit into
gtk-rs:mainfrom
A6GibKm:safe-io-3
Open

unix_fd_add: Port to safe-io#1745
A6GibKm wants to merge 1 commit into
gtk-rs:mainfrom
A6GibKm:safe-io-3

Conversation

@A6GibKm

@A6GibKm A6GibKm commented Jun 21, 2025

Copy link
Copy Markdown
Contributor

From #871.

@A6GibKm

A6GibKm commented Jun 21, 2025

Copy link
Copy Markdown
Contributor Author

From #871 (comment):

@sdroege: I think these functions all need to be unsafe actually. impl AsFd is correct but it's up to the caller to ensure that the fd is valid long enough

In that case would it not be better to pass an OwnedFd? The docs for unix_fd_add or unix_fd_add_full don't say much.

sdroege
sdroege previously approved these changes Jun 21, 2025
@sdroege

sdroege commented Jun 21, 2025

Copy link
Copy Markdown
Member

In that case would it not be better to pass an OwnedFd? The docs for unix_fd_add or unix_fd_add_full don't say much.

Good question. But can you get an OwnedFd for stdin, for example?

@sdroege

sdroege commented Jun 21, 2025

Copy link
Copy Markdown
Member

Also, shouldn't the GSource trigger when it's closed and be destroyed then? From inside GLib I mean.

@A6GibKm

A6GibKm commented Jun 21, 2025

Copy link
Copy Markdown
Contributor Author

In that case would it not be better to pass an OwnedFd? The docs for unix_fd_add or unix_fd_add_full don't say much.

Good question. But can you get an OwnedFd for stdin, for example?

If you mean std::io::Stdin then it implements AsFd from which you can call https://doc.rust-lang.org/std/os/fd/struct.BorrowedFd.html#method.try_clone_to_owned.

@A6GibKm A6GibKm mentioned this pull request Jun 21, 2025
11 tasks
@sdroege

sdroege commented Jun 25, 2025

Copy link
Copy Markdown
Member

I think the main question here is what happens with the GSource if the fd is closed. Random things? Does the GSource get destroyed cleanly?

@sdroege

sdroege commented Dec 14, 2025

Copy link
Copy Markdown
Member

@A6GibKm This would need to be handled in the next weeks if it should end up in the next release

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