Skip to content

Conversation

epilys
Copy link
Contributor

@epilys epilys commented Dec 24, 2024

Since 1.70.0, the SocketAddrExt trait provides ways to create a
socket address for Unix sockets with abstract names: https://doc.rust-lang.org/std/os/linux/net/trait.SocketAddrExt.html

Since 1.70.0, the SocketAddrExt trait [0] provides ways to create a
socket address for Unix sockets with abstract names.

[0]: https://doc.rust-lang.org/std/os/linux/net/trait.SocketAddrExt.html

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
When creating a new Unix socket, don't allocate a new String needlessly.

Signed-off-by: Manos Pitsidianakis <manos@pitsidianak.is>
@coveralls
Copy link

Pull Request Test Coverage Report for Build 12485070824

Details

  • 14 of 14 (100.0%) changed or added relevant lines in 2 files are covered.
  • 3 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.003%) to 57.26%

Files with Coverage Reduction New Missed Lines %
varlink/src/client.rs 3 28.79%
Totals Coverage Status
Change from base Build 12119901637: 0.003%
Covered Lines: 2784
Relevant Lines: 4862

💛 - Coveralls

Copy link
Collaborator

@zeenix zeenix left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks so much!

@zeenix zeenix merged commit c2908aa into varlink:master Dec 25, 2024
14 checks passed
@epilys epilys deleted the remove-unix_socket-dep branch December 25, 2024 17:53
@zeenix
Copy link
Collaborator

zeenix commented Dec 27, 2024

@epilys Seems the CI in master is failing consistently after this PR was merged. Could you please have a look?

@epilys
Copy link
Contributor Author

epilys commented Dec 27, 2024

@zeenix it seems like a flaky test? The PR CI didn't timeout on test_unix_multiplex but the merge event CI run did. I will take a look

@epilys
Copy link
Contributor Author

epilys commented Dec 27, 2024

@zeenix FWIW I was able to get the exact same test timeout on the commit before this PR was merged by running it a bunch of times locally.

@zeenix
Copy link
Collaborator

zeenix commented Jan 6, 2025

@zeenix FWIW I was able to get the exact same test timeout on the commit before this PR was merged by running it a bunch of times locally.

Oh interesting. Will have a look when/if it comes up failing again. If you have the time, please do have a look into it. 👍

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