Skip to content

Conversation

@bugaevc
Copy link
Contributor

@bugaevc bugaevc commented May 19, 2017

We should also add AT_EACCESS (and the rest of AT_* flags), but that's not included in this PR.

}
}

pub enum AccessMode {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be documented as it's a public interface.

}

libc_bitflags!{
pub flags AccessFlags: c_int {
Copy link
Contributor

Choose a reason for hiding this comment

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

Please document this.

src/unistd.rs Outdated
use {Errno, Error, Result, NixPath};
use fcntl::{fcntl, OFlag, O_CLOEXEC, FD_CLOEXEC};
use fcntl::{fcntl, OFlag, AtFlags, O_CLOEXEC, FD_CLOEXEC};
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be alphabetical, so AtFlags is first.

Flags(AccessFlags),
}

impl AccessMode {
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be commented.

src/unistd.rs Outdated
}
}

/// Check whether a file exists or whether the current process can access a file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Please link to the man page for this. See the fork() function doccomments for an example.

Errno::result(res).map(drop)
}

/// Check whether a file exists or whether the current process can access a file,
Copy link
Contributor

Choose a reason for hiding this comment

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

Please link to the man page for this. See the fork() function doccomments for an example.

@Susurrus
Copy link
Contributor

Thanks for your PR, @bugaevc! I imagine this is pretty useful. I've made some comments, mostly superficial stuff right now. I'll come back and make a deeper inspection of stuff once things are a little more comments. Also, when you make these changes, could you squash everything into 1 commit? Thanks!

@bugaevc
Copy link
Contributor Author

bugaevc commented May 20, 2017

☑️ Done

@Susurrus
Copy link
Contributor

Susurrus commented Jun 5, 2017

I just realized that this PR overlaps with #562, which covers a lot more ground. I'd like to close this PR in favor of that one. Would you be willing to help me review that PR and we can close this one?

@Susurrus Susurrus mentioned this pull request Jun 5, 2017
@bugaevc
Copy link
Contributor Author

bugaevc commented Jun 5, 2017

Sure -- I wonder how come I haven't noticed it myself.

@Susurrus Susurrus closed this Jun 5, 2017
@oblique oblique mentioned this pull request Dec 12, 2018
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