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

added documentation for getcwd #326

Closed
wants to merge 5 commits into from
Closed

added documentation for getcwd #326

wants to merge 5 commits into from

Conversation

philippkeller
Copy link

I'm currently reading through Advanced Programming in the UNIX Environment which goes through most of the libc functions, explains them and does some example code. I'm trying to port the code into Rust.

I thought about adding my findings as documentation string into libc. Would that be a thing that would benefit this repo? I've added the documentation for getcwd in this PR, would that be good enough? I would add those documentation bits by bits. Is that ok or should I put as much documentation as possible in one huge PR?

@rust-highfive
Copy link

r? @alexcrichton

(rust_highfive has picked a reviewer for you, use r? to override)

@alexcrichton
Copy link
Member

Thanks for the PR! Right now though this crate is basically just a massive header file, and we probably don't want to go down the road of adding an example for all the functions bound here.

This does seem like some good documentation, though! @steveklabnik do you have any idea perhaps on where documentation like this might go?

@posborne
Copy link
Contributor

Odd timing, I've recently been thinking about docs and was just wrapping up a PR starting to improve the situation for nix (nix-rust/nix#385). As can be seen in the getcwd example, writing code that uses the libc APIs correctly can be a task.

@philippkeller It looks like we don't currently have support for getcwd in nix. Would you be interested in adding support (with docs) there? The goal of nix is to wrap the underlying libc APIs (mostly 1-to-1 but varying where required to safely capture semantics).

In general, I think it would be preferable for people to be encourage to use either std, nix, or some other more specialized crate for interacting with libc functionality rather than using libc directly. That being said, having better documentation on libc and the aforementioned advice are not mutually exclusive.

@philippkeller
Copy link
Author

@posborne: sure! I am new to Rust though and if I would contribute code it needs some attentive pair of eyes on my PRs. If that is ok then I'd be happy to contribute. I was not really sure if the documentation really belongs here (as @alexcrichton says it's just a header file and I was not sure if that was intentional or not).

That said, @posborne if you could tell where getcwd should be living within nix, I'll retract my PR here and transform the PR into a nix PR with code and documentation.

@posborne
Copy link
Contributor

That said, @posborne if you could tell where getcwd should be living within nix, I'll retract my PR here and transform the PR into a nix PR with code and documentation.

Currently, we generally map the libc header files (in this case unistd.h) to modules in the library, so unistd.rs.

@steveklabnik
Copy link
Member

This does seem like some good documentation, though! @steveklabnik do you have any idea perhaps on where documentation like this might go?

This is a good question. I'm not sure; I'm not totally opposed to adding docs here, but can also see why we might not want to.

@philippkeller
Copy link
Author

@posborne one question before I move this to nix: getcwd is already implemented in std which is exposed via env::current_dir. Does it make sense to have it also implemented in nix?

@posborne
Copy link
Contributor

@posborne one question before I move this to nix: getcwd is already implemented in std which is exposed via env::current_dir. Does it make sense to have it also implemented in nix?

The std implementation is probably what users should use but I think it would be OK to add to nix as well for completeness/convenience (if code was using other code from Nix extensively or working in a no_std environment it might make sense).

CC @fiveop @kamalmarhubi

@fiveop
Copy link
Contributor

fiveop commented Jul 12, 2016

I agree with @posborne in that we (nix) should aim for completeness, even in cases were we don't see the necessity.

And, even though I have no stake in this, I would think documentation for libc would be a bit redundant to the corresponding man pages. However, nix does change the interface sufficiently - even if only slightly - and has at least somewhat organized code, so that having its own documentation would be useful in my opinion.

homu added a commit to nix-rust/nix that referenced this pull request Sep 7, 2016
add unistd::getcwd and unistd::mkdir

As a (late) followup of [this withdrawn PR](rust-lang/libc#326) I have added getcwd (wrapper around `libc::getcwd`) and mkdir (wrapper around `libc::mkdir`) and added testing.

A few notes:

 - I'm new to rust so I would appreciate some pair of eyes testing the code, plus I'm open for revision of code or general remarks about my coding style
 - I have run the tests both on OSX as on Linux (Ubuntu)
 - I've run `clippy` to see if my code is well formatted, however clippy issues many warnings about the project. I think I didn't add any more warnings
 - the methods in unistd are not documented so I also left out the documentation of `getcwd` and `mkdir`, although I think it'd probably be good to add some documentation, especially some example code how to use the methods
 - the base idea of `getcwd` is [taken from std](https://github.com/rust-lang/rust/blob/1.9.0/src/libstd/sys/unix/os.rs#L95-L119), should I mention that somewhere?
Susurrus pushed a commit to Susurrus/libc that referenced this pull request Mar 26, 2017
…hubi

Fix arm and x86 ci

See the individual commit message for more detail.  ARM is now passing (but we don't run mount tests any more when in the docker container).  For x86_64/i686, there were some changes in the posborne/rust-cross stuff.
danielverkamp pushed a commit to danielverkamp/libc that referenced this pull request Apr 28, 2020
* implemented rdrand and rdseed intrinsics

* added "unsigned short*" case

* moved rdrand from i686 to x86_64

* 64 bit rdrand functions in x86_64, 16 and 32 in i686
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.

None yet

6 participants