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

fix: replaced ffi getuid by posix package #360

Closed
wants to merge 1 commit into from

Conversation

aadarshadhakalg
Copy link

Fixes: #191

Use posix package to get uid

@github-actions
Copy link

github-actions bot commented Jun 4, 2023

Hey! aadarshadhakalg has not signed the Canonical CLA which is required to get this contribution merged on this project.

Please head over to https://ubuntu.com/legal/contributors to read more about it.

@robert-ancell
Copy link
Contributor

The posix package wasn't as established at the time but it seems to be now. How does this handle when running on Windows? @jpnurmi any problems doing this you know of?

@aadarshadhakalg
Copy link
Author

It seems like the posix package doesnot have windows getsid() implementation, so for windows it still uses existing ffi implementation.

@jpnurmi
Copy link
Contributor

jpnurmi commented Jun 7, 2023

There's also "our own" stdlibc but it doesn't support Windows either. Anyway, is it worth adding any library dependency for that one function? Using dart:ffi directly wouldn't tie dbus.dart to the release schedule of another package...

@aadarshadhakalg
Copy link
Author

If we can add windows implementation to stdlibc then using stdlibc would be a nice. If we can't do that and we must use ffi to support windows, then adding an extra dependency is not good and we can close this issue I guess.

@robert-ancell
Copy link
Contributor

I'm going to agree here with @jpnurmi - the cost of the dependency doesn't exceeds any benefit of removing the code. Let's stick with what we have.

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.

Replace FFI getuid
3 participants