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.

Sign up for GitHub

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

Jump to bottom

UnixFD: Move the support for Unix file descriptors to dasbus.unix #100

Merged
merged 4 commits into from
Oct 11, 2022
Merged

UnixFD: Move the support for Unix file descriptors to dasbus.unix #100

merged 4 commits into from
Oct 11, 2022

Conversation

poncovka
Copy link
Contributor

@poncovka poncovka commented Oct 4, 2022 •
edited
Loading

  • Move all functions and classes related to the UnixFD support to dasbus.unix.
  • Clean up the implementation of GLibClientUnix and GLibServerUnix.
  • Clean up tests for swapping Unix file descriptors.

@poncovka
UnixFD: Move the support for Unix file descriptors to dasbus.unix
a295cd6 
Move all functions and classes related to the UnixFD support to dasbus.unix.
@poncovka
UnixFD: Process results of client calls in the low-level library
2b23725 
The `GLibClientUnix` class should be able to handle the result of a client
call that might contain Unix file descriptors. The `ClientObjectHandler`
class shouldn't support anything related to that.
@poncovka
UnixFD: Clean up GLibClientUnix and GLibServerUnix
d725bd7 
* Remove the `variant_replace_fdlist_indices_with_handles` function.
* Remove the `variant_replace_handles_with_fdlist_indices` function.
* Use the `acquire_fds` and `restore_fds` functions instead.
@poncovka
UnixFD: Clean up tests for swapping Unix file descriptors
427dbb7 
Clean up unit tests for the `UnixFDSwap` class and the related code.
@poncovka poncovka added the refactoring Clean up of the code label Oct 4, 2022
@poncovka poncovka mentioned this pull request Oct 7, 2022
Merged
M4rtinK
M4rtinK approved these changes Oct 10, 2022
View reviewed changes
Copy link

@M4rtinK M4rtinK left a comment

Choose a reason for hiding this comment

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

Looks good to me, thanks! :)

VladimirSlavik
VladimirSlavik approved these changes Oct 10, 2022
View reviewed changes
Copy link

@VladimirSlavik VladimirSlavik left a comment

Choose a reason for hiding this comment

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

Looks good to me.

I suggest naming the module and things unix_fd or just fd - the "unix" bit is not exactly bringing any information. You already named the individual tests exactly that way, so it seems to be a sounds logic.

dasbus/unix.py Show resolved Hide resolved
@poncovka
Copy link
Contributor Author

I suggest naming the module and things unix_fd or just fd - the "unix" bit is not exactly bringing any information. You already named the individual tests exactly that way, so it seems to be a sounds logic.

I want to use the dasbus.unix namespace for all unix-specific code. In the future, there might be dasbus.unix.fd, but it is not necessary at this moment. This code will not work on non-unix systems, so I hope people will think twice before they will import anything from dasbus.unix.

@poncovka poncovka merged commit 25f24b7 into dasbus-project:master Oct 11, 2022
@VladimirSlavik
Copy link

Oh. I didn't know D-Bus is available outside Linux. In that case it makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@VladimirSlavik VladimirSlavik VladimirSlavik approved these changes

@M4rtinK M4rtinK M4rtinK approved these changes

Assignees
No one assigned
Labels
refactoring Clean up of the code
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@poncovka @VladimirSlavik @M4rtinK