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

add filedescriptor class #21

Merged
merged 1 commit into from
Nov 21, 2024
Merged

add filedescriptor class #21

merged 1 commit into from
Nov 21, 2024

Conversation

gulafaran
Copy link
Contributor

@gulafaran gulafaran commented Nov 20, 2024

would be neat to slowly consume up all the manual handling of fd closing and using something like this, but needs some sharper eyes on the various constructors and operators. got to say i lack there

based on kwin's idea of FD handling.

include/hyprutils/utils/FileDescriptor.hpp Outdated Show resolved Hide resolved
include/hyprutils/utils/FileDescriptor.hpp Outdated Show resolved Hide resolved
include/hyprutils/utils/FileDescriptor.hpp Outdated Show resolved Hide resolved
src/utils/FileDescriptor.cpp Outdated Show resolved Hide resolved
src/utils/FileDescriptor.cpp Outdated Show resolved Hide resolved
src/utils/FileDescriptor.cpp Outdated Show resolved Hide resolved
src/utils/FileDescriptor.cpp Outdated Show resolved Hide resolved
Copy link
Member

@vaxerski vaxerski left a comment

Choose a reason for hiding this comment

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

misses a test case, rest lgtm

@gulafaran
Copy link
Contributor Author

misses a test case, rest lgtm

added a test case, think im testing most of the things. used shm_open as an example

@gulafaran gulafaran changed the title draft: filedescriptor class add filedescriptor class Nov 21, 2024
@vaxerski
Copy link
Member

last thing, I'd move that under OS rather than utils.

makes you able to RAII the filedescriptor making it somewhat easier to
not leak.
@gulafaran
Copy link
Contributor Author

last thing, I'd move that under OS rather than utils.

moved

@vaxerski vaxerski merged commit 2e21319 into hyprwm:main Nov 21, 2024
4 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.

2 participants