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

Lots of libc #1

Open
Arcterus opened this issue Mar 30, 2018 · 4 comments
Open

Lots of libc #1

Arcterus opened this issue Mar 30, 2018 · 4 comments

Comments

@Arcterus
Copy link

Is there a reason why there are so many calls to libc functions when there are equivalent functions in safe Rust? For example it seems like libc::setenv() could be replaced with env::set_var() and libc::chdir() could be replaced with env::set_current_dir().

@mssun
Copy link
Contributor

mssun commented Mar 30, 2018

Thanks, @Arcterus. I don't have a very strong reason to only use libc functions. But I do meet following problems:

  • nix (https://github.com/nix-rust/nix) is a good wrapper for libc functions, but it's not completed . Some function are still missing (e.g., getpwnam is missing nix-rust/nix#862)
  • std functions are good, but we still need libc for others
  • I'm not sure libc::setenv() and env::set_var() have exact same function (they are probably same). If not same, I don't know if using std will bring side effects.

If I try my best to avoid libc and use nix and std, the code will mix everything together which looks ugly. I tend to use nix and std functions only. But before nix is completed, I have to use libc.

Anyway, for this case (libc::setenv() vs env::set_var()), I read the code https://github.com/rust-lang/rust/blob/master/src/libstd/sys/unix/os.rs#L467. You are right, I think it's better to use env::set_var. (At least from the perspective of maintenance, but looks like it uses extra memory)

If you have other suggestion, feel free to comment. Many thanks.

@Arcterus
Copy link
Author

Usually I think it's better to use std when possible, as that way we can reduce the amount of unsafe code and make it easier to track down related bugs. I can see how the allocations may be an issue though (set_current_dir() similarly seems to allocate to handle the C string). My goal with this issue is basically to lessen the amount of code that requires unsafe.

A better way to replace the libc stuff might be to take the libc::chdir(), libc::setenv()s, and libc::execl() and replace them with Command (using .current_dir(), .env(), and .exec()). However, I assume they will also allocate to make sure the given strings have an ending null byte.

@mssun
Copy link
Contributor

mssun commented Apr 2, 2018

Agree. I saw you fork the repo and made some modification. Free feel to send PRs.

@Arcterus
Copy link
Author

Arcterus commented Apr 2, 2018

My fork won't compile yet because I need to get some changes merged into rust-users (so I can access the user's password).

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

No branches or pull requests

2 participants