Documentation for Whence. #730

hilias · 2017-08-10T16:49:32Z

No description provided.

Susurrus · 2017-08-10T16:56:36Z

src/unistd.rs

 pub enum Whence {
+    /// The file offset shall be set to offset bytes.


"Specify an offset relative to the start of the file"

Susurrus · 2017-08-10T16:57:39Z

src/unistd.rs

    SeekSet,
+    /// The file offset shall be set to its current location plus offset.


"Specify an offset relative to the current file location"

Susurrus · 2017-08-10T16:58:00Z

src/unistd.rs

    SeekCur,
+    /// The file offset shall be set to the size of the file plus offset.


"Specify an offset relative to the end of the file"

Susurrus · 2017-08-10T17:01:17Z

src/unistd.rs

    SeekEnd,
+    /// Adjust the file offset to the next location in the file greater than


"Specify a file offset relative to the next location in the file greater than or equal to offset containing data. If offset points to data, then the file offset is set to offset."

That being said I really don't understand what "data" is in this context and how "offset" relates to it.

Data is supposed to mean a part of the file that's been written to. Maybe it should be called 'some data'?

Susurrus · 2017-08-10T17:01:52Z

src/unistd.rs

    SeekData,
+    /// Adjust the file offset to the next hole in the file greater than or


Again, this only "specifies", it doesn't "adjust". The *seek() functions adjust, these constants are just specifying a position.

Susurrus · 2017-08-10T17:04:40Z

src/unistd.rs

@@ -737,11 +737,23 @@ pub fn write(fd: RawFd, buf: &[u8]) -> Result<usize> {
    Errno::result(res).map(|r| r as usize)
 }

+/// Directive that tells lseek how to reposition the offset of the file.


Please reference lseek and lseek64 and specify them as Markdown links to the documentation. See here for an example. You'll need to figure out the exact paths for lseek/lseek64, but the premise is the same.

Susurrus · 2017-08-10T23:20:38Z

src/unistd.rs

@@ -737,11 +737,25 @@ pub fn write(fd: RawFd, buf: &[u8]) -> Result<usize> {
    Errno::result(res).map(|r| r as usize)
 }

+/// Directive that tells [`lseek`] and [`lseek64`] how to reposition the offset of the file.


"Directive for [lseek] and [lseek64] that specifies what the offset is relative to."

Susurrus · 2017-08-10T23:21:20Z

If you're planning on tackling all of #676, we can keep that as part of this same PR as additional commits.

Susurrus · 2017-08-11T01:54:21Z

Can you squash these commits all into one (lookup the git rebase command if you're unfamiliar with doing this)?

Otherwise are you planning on adding the other parts of #676 to this PR or we just going to merge the documentation part in this one?

hilias · 2017-08-11T02:32:18Z

I was thinking on tackling all of #676. I have a question with the conditional availabilty part, SEEK_DATA and SEEK_HOLE are defined on libc only on the linux module, so should I consider that the only platform where they are available?

Susurrus · 2017-08-11T02:57:34Z

You'll want to check all applicable platforms. They should at least be added to DragonFlyBSD (declared here). FreeBSD also has them. It looks like it's not in uclibc or linux/musl. So I think those platforms are the only two missing it. I checked NetBSD, OpenBSD, and Mac and they don't have either of them, so that's fine.

Susurrus · 2017-08-11T03:41:04Z

@ncaracci You'll need to add these constants to libc before this will work. You should file a PR against libc adding these constants here. Then your changes will work.

Susurrus · 2017-08-11T03:44:21Z

src/unistd.rs

@@ -751,8 +767,10 @@ impl Whence {
            &Whence::SeekSet => libc::SEEK_SET,
            &Whence::SeekCur => libc::SEEK_CUR,
            &Whence::SeekEnd => libc::SEEK_END,
-            &Whence::SeekData => 3,
-            &Whence::SeekHole => 4
+            #[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd"))]


Don't do this here, delete this whole impl and just assign values within the enum like SeekSet = libc::SEEK_SET,. You'll also want to say that it's a i32 datatype. Look at how it's implemented for Signal.

Susurrus · 2017-08-11T05:03:46Z

src/unistd.rs

+    /// Specify an offset relative to the next location in the file greater than or 
+    /// equal to offset that contains some data. If offset points to
+    /// some data, then the file offset is set to offset.
+    #[cfg(any(target_os = "linux", target_os = "dragonfly", target_os = "freebsd"))]


Please alphabetize the OS names here and below.

hilias · 2017-08-14T04:27:30Z

Would adding not(target_env="musl", target_arch="mips") to the cfg fix the build errors?

Susurrus · 2017-08-14T14:38:38Z

Use a whitelist for Linux instead: #[cfg(any(target_os = "dragonfly", target_os = "freebsd", all(target_os = "linux", not(any(target_env = "musl", target_arch = "mips"))))]

Susurrus · 2017-08-14T19:28:28Z

You'll need to exclude arch mip64 as well.

Changed SeekData and SeekHole hardcoded values to libc ones and added availability only to linux freebsd and dragonflybsd. Removed impl Whence and assigned the libc values directly in the enum definition.

asomers · 2017-08-15T04:20:27Z

Thanks for the improvements, @ncaracci

bors r+

730: Documentation for Whence. r=asomers

bors · 2017-08-15T08:26:42Z

Timed out

asomers · 2017-08-15T14:55:09Z

bors timed out because of a known bug in buildbot, triggered by the non-ascii character in the author's name. I'll manually merge it.

hilias mentioned this pull request Aug 10, 2017

Improve unistd::Whence #676

Closed

4 tasks

Susurrus suggested changes Aug 10, 2017

View reviewed changes

Susurrus approved these changes Aug 11, 2017

View reviewed changes

Susurrus suggested changes Aug 11, 2017

View reviewed changes

Susurrus approved these changes Aug 11, 2017

View reviewed changes

hilias force-pushed the whence branch from 3c7747e to 13cbae0 Compare August 11, 2017 12:12

Added documentation to Whence.

3d7d7ea

Changed SeekData and SeekHole hardcoded values to libc ones and added availability only to linux freebsd and dragonflybsd. Removed impl Whence and assigned the libc values directly in the enum definition.

hilias force-pushed the whence branch from e562c11 to 3d7d7ea Compare August 15, 2017 01:14

bors bot added a commit that referenced this pull request Aug 15, 2017

Merge #730

eb9a30a

730: Documentation for Whence. r=asomers

asomers merged commit fb329e8 into nix-rust:master Aug 15, 2017

hilias deleted the whence branch August 15, 2017 14:59

		pub enum Whence {
		/// The file offset shall be set to offset bytes.

		SeekSet,
		/// The file offset shall be set to its current location plus offset.

		SeekCur,
		/// The file offset shall be set to the size of the file plus offset.

		SeekEnd,
		/// Adjust the file offset to the next location in the file greater than

		SeekData,
		/// Adjust the file offset to the next hole in the file greater than or

Documentation for Whence. #730

Documentation for Whence. #730

Uh oh!

Conversation

hilias commented Aug 10, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Susurrus commented Aug 10, 2017

Uh oh!

Susurrus commented Aug 11, 2017

Uh oh!

hilias commented Aug 11, 2017

Uh oh!

Susurrus commented Aug 11, 2017

Uh oh!

Susurrus commented Aug 11, 2017

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

hilias commented Aug 14, 2017 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Susurrus commented Aug 14, 2017

Uh oh!

Susurrus commented Aug 14, 2017

Uh oh!

asomers commented Aug 15, 2017

Uh oh!

bors bot commented Aug 15, 2017

Timed out

Uh oh!

asomers commented Aug 15, 2017

Uh oh!

Uh oh!

hilias commented Aug 14, 2017 •

edited

Loading