From ea102096803dd12acf5fc592fb5c581e9b9ff51d Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 13:17:02 +0200 Subject: [PATCH 1/8] fix readlink/readlinkat to return too long only when it is long --- src/fcntl.rs | 4 +++- test/test_fcntl.rs | 19 +++++++++++++++---- 2 files changed, 18 insertions(+), 5 deletions(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index be6ee0f73a..f590e361d8 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -181,7 +181,9 @@ fn wrap_readlink_result(buffer: &mut[u8], res: ssize_t) -> Result<&OsStr> { match Errno::result(res) { Err(err) => Err(err), Ok(len) => { - if (len as usize) >= buffer.len() { + if len < 0 { + Err(Error::Sys(Errno::EINVAL)) + } else if (len as usize) > buffer.len() { Err(Error::Sys(Errno::ENAMETOOLONG)) } else { Ok(OsStr::from_bytes(&buffer[..(len as usize)])) diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index 6b2bbd679f..c8773219f4 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -56,12 +56,23 @@ fn test_readlink() { let dirfd = open(tempdir.path(), OFlag::empty(), Mode::empty()).unwrap(); + let expected_dir = src.to_str().unwrap(); + // When the size of the buffer is bigger than the expected directory length let mut buf = vec![0; src.to_str().unwrap().len() + 1]; - assert_eq!(readlink(&dst, &mut buf).unwrap().to_str().unwrap(), - src.to_str().unwrap()); - assert_eq!(readlinkat(dirfd, "b", &mut buf).unwrap().to_str().unwrap(), - src.to_str().unwrap()); + assert_eq!(readlink(&dst, &mut buf).unwrap().to_str().unwrap(), expected_dir); + assert_eq!(readlinkat(dirfd, "b", &mut buf).unwrap().to_str().unwrap(), expected_dir); + + // When the size of the buffer is equal to the expected directory length + let mut exact_buf = vec![0; src.to_str().unwrap().len()]; + assert_eq!(readlink(&dst, &mut exact_buf).unwrap().to_str().unwrap(), expected_dir); + assert_eq!(readlinkat(dirfd, "b", &mut exact_buf).unwrap().to_str().unwrap(), expected_dir); + + // When the size of the buffer is smaller than the expected directory length + let mut small_buf = vec![0;0]; + assert_eq!(readlink(&dst, &mut small_buf).unwrap().to_str().unwrap(), ""); + assert_eq!(readlinkat(dirfd, "b", &mut small_buf).unwrap().to_str().unwrap(), ""); + } #[cfg(any(target_os = "linux", target_os = "android"))] From e7433361a4312a04be7a9d0250fffb99bfce45bc Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 20:50:13 +0200 Subject: [PATCH 2/8] update readlink to return the path instead of mangling the buffer --- src/fcntl.rs | 33 +++++++++++++++------------------ test/test_fcntl.rs | 16 ++-------------- test/test_unistd.rs | 5 ++--- 3 files changed, 19 insertions(+), 35 deletions(-) diff --git a/src/fcntl.rs b/src/fcntl.rs index f590e361d8..2adbb6ab4f 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -1,11 +1,11 @@ -use {Error, Result, NixPath}; +use {Result, NixPath}; use errno::Errno; use libc::{self, c_int, c_uint, c_char, size_t, ssize_t}; use sys::stat::Mode; use std::os::raw; use std::os::unix::io::RawFd; -use std::ffi::OsStr; -use std::os::unix::ffi::OsStrExt; +use std::ffi::OsString; +use std::os::unix::ffi::OsStringExt; #[cfg(any(target_os = "android", target_os = "linux"))] use std::ptr; // For splice and copy_file_range @@ -177,36 +177,33 @@ pub fn renameat(old_dirfd: Option Result<&OsStr> { +fn wrap_readlink_result(v: &mut Vec, res: ssize_t) -> Result { match Errno::result(res) { Err(err) => Err(err), Ok(len) => { - if len < 0 { - Err(Error::Sys(Errno::EINVAL)) - } else if (len as usize) > buffer.len() { - Err(Error::Sys(Errno::ENAMETOOLONG)) - } else { - Ok(OsStr::from_bytes(&buffer[..(len as usize)])) - } + unsafe { v.set_len(len as usize) } + Ok(OsString::from_vec(v.to_vec())) } } } -pub fn readlink<'a, P: ?Sized + NixPath>(path: &P, buffer: &'a mut [u8]) -> Result<&'a OsStr> { +pub fn readlink<'a, P: ?Sized + NixPath>(path: &P) -> Result { + let mut v = vec![0u8; libc::PATH_MAX as usize]; let res = path.with_nix_path(|cstr| { - unsafe { libc::readlink(cstr.as_ptr(), buffer.as_mut_ptr() as *mut c_char, buffer.len() as size_t) } + unsafe { libc::readlink(cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.len() as size_t) } })?; - - wrap_readlink_result(buffer, res) + + wrap_readlink_result(&mut v, res) } -pub fn readlinkat<'a, P: ?Sized + NixPath>(dirfd: RawFd, path: &P, buffer: &'a mut [u8]) -> Result<&'a OsStr> { +pub fn readlinkat<'a, P: ?Sized + NixPath>(dirfd: RawFd, path: &P) -> Result { + let mut v = vec![0u8; libc::PATH_MAX as usize]; let res = path.with_nix_path(|cstr| { - unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), buffer.as_mut_ptr() as *mut c_char, buffer.len() as size_t) } + unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.len() as size_t) } })?; - wrap_readlink_result(buffer, res) + wrap_readlink_result(&mut v, res) } /// Computes the raw fd consumed by a function of the form `*at`. diff --git a/test/test_fcntl.rs b/test/test_fcntl.rs index c8773219f4..1bcf12cb5b 100644 --- a/test/test_fcntl.rs +++ b/test/test_fcntl.rs @@ -58,20 +58,8 @@ fn test_readlink() { Mode::empty()).unwrap(); let expected_dir = src.to_str().unwrap(); - // When the size of the buffer is bigger than the expected directory length - let mut buf = vec![0; src.to_str().unwrap().len() + 1]; - assert_eq!(readlink(&dst, &mut buf).unwrap().to_str().unwrap(), expected_dir); - assert_eq!(readlinkat(dirfd, "b", &mut buf).unwrap().to_str().unwrap(), expected_dir); - - // When the size of the buffer is equal to the expected directory length - let mut exact_buf = vec![0; src.to_str().unwrap().len()]; - assert_eq!(readlink(&dst, &mut exact_buf).unwrap().to_str().unwrap(), expected_dir); - assert_eq!(readlinkat(dirfd, "b", &mut exact_buf).unwrap().to_str().unwrap(), expected_dir); - - // When the size of the buffer is smaller than the expected directory length - let mut small_buf = vec![0;0]; - assert_eq!(readlink(&dst, &mut small_buf).unwrap().to_str().unwrap(), ""); - assert_eq!(readlinkat(dirfd, "b", &mut small_buf).unwrap().to_str().unwrap(), ""); + assert_eq!(readlink(&dst).unwrap().to_str().unwrap(), expected_dir); + assert_eq!(readlinkat(dirfd, "b").unwrap().to_str().unwrap(), expected_dir); } diff --git a/test/test_unistd.rs b/test/test_unistd.rs index 46196dec7c..e1e03f3f93 100644 --- a/test/test_unistd.rs +++ b/test/test_unistd.rs @@ -576,14 +576,13 @@ fn test_canceling_alarm() { #[test] fn test_symlinkat() { - let mut buf = [0; 1024]; let tempdir = tempfile::tempdir().unwrap(); let target = tempdir.path().join("a"); let linkpath = tempdir.path().join("b"); symlinkat(&target, None, &linkpath).unwrap(); assert_eq!( - readlink(&linkpath, &mut buf).unwrap().to_str().unwrap(), + readlink(&linkpath).unwrap().to_str().unwrap(), target.to_str().unwrap() ); @@ -592,7 +591,7 @@ fn test_symlinkat() { let linkpath = "d"; symlinkat(target, Some(dirfd), linkpath).unwrap(); assert_eq!( - readlink(&tempdir.path().join(linkpath), &mut buf) + readlink(&tempdir.path().join(linkpath)) .unwrap() .to_str() .unwrap(), From 3d5eadde40c8760c61afe5c1912247efd1b53453 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 20:53:39 +0200 Subject: [PATCH 3/8] Add changelog --- CHANGELOG.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index d93a5ce6bb..a930b57765 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - ReleaseDate ### Added ### Changed +- Changed `readlink` and `readlinkat` to return `osString` + ([#1109](https://github.com/nix-rust/nix/pull/1109)) ### Fixed ### Removed From 07e2ea98d8b97bbf33ce96ef4dd1a1d6929e5773 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 21:44:16 +0200 Subject: [PATCH 4/8] fix review comments --- CHANGELOG.md | 9 ++++++++- src/fcntl.rs | 12 +++++++----- 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a930b57765..ef6538d07b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,8 +6,15 @@ This project adheres to [Semantic Versioning](http://semver.org/). ## [Unreleased] - ReleaseDate ### Added ### Changed -- Changed `readlink` and `readlinkat` to return `osString` +- Changed `readlink` and `readlinkat` to return `OsString` ([#1109](https://github.com/nix-rust/nix/pull/1109)) + + ```rust + use nix::fcntl::readlink; + + readlink!(&path); + ``` + ### Fixed ### Removed diff --git a/src/fcntl.rs b/src/fcntl.rs index 2adbb6ab4f..1ef26271ab 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -188,19 +188,21 @@ fn wrap_readlink_result(v: &mut Vec, res: ssize_t) -> Result { } pub fn readlink<'a, P: ?Sized + NixPath>(path: &P) -> Result { - let mut v = vec![0u8; libc::PATH_MAX as usize]; + let len = libc::PATH_MAX as usize; + let mut v = Vec::with_capacity(len); let res = path.with_nix_path(|cstr| { - unsafe { libc::readlink(cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.len() as size_t) } + unsafe { libc::readlink(cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, len as size_t) } })?; - + wrap_readlink_result(&mut v, res) } pub fn readlinkat<'a, P: ?Sized + NixPath>(dirfd: RawFd, path: &P) -> Result { - let mut v = vec![0u8; libc::PATH_MAX as usize]; + let len = libc::PATH_MAX as usize; + let mut v = Vec::with_capacity(len); let res = path.with_nix_path(|cstr| { - unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.len() as size_t) } + unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, len as size_t) } })?; wrap_readlink_result(&mut v, res) From e99c267f707776ff4dcc786d5bb2987a3546bfdf Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 21:49:31 +0200 Subject: [PATCH 5/8] add readlinkat too --- CHANGELOG.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef6538d07b..6d060eda4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,9 +10,10 @@ This project adheres to [Semantic Versioning](http://semver.org/). ([#1109](https://github.com/nix-rust/nix/pull/1109)) ```rust - use nix::fcntl::readlink; + use nix::fcntl::{readlink, readlinkat}; readlink!(&path); + readlinkat!(dirfd, &path); ``` ### Fixed From 67375852b45636c251faeb2e6be43bdf40d709a4 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 21:50:20 +0200 Subject: [PATCH 6/8] remove macro symbol --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 6d060eda4c..f3ea037f79 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,8 @@ This project adheres to [Semantic Versioning](http://semver.org/). ```rust use nix::fcntl::{readlink, readlinkat}; - readlink!(&path); - readlinkat!(dirfd, &path); + readlink(&path); + readlinkat(dirfd, &path); ``` ### Fixed From 4a4cfc0b4ed9bb30c0be09d8d7e49b8fb8e2e805 Mon Sep 17 00:00:00 2001 From: Sendil Kumar Date: Thu, 22 Aug 2019 22:22:29 +0200 Subject: [PATCH 7/8] updated changelog --- CHANGELOG.md | 12 ++++++++++-- src/fcntl.rs | 10 ++++------ 2 files changed, 14 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f3ea037f79..2765c33349 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,8 +12,16 @@ This project adheres to [Semantic Versioning](http://semver.org/). ```rust use nix::fcntl::{readlink, readlinkat}; - readlink(&path); - readlinkat(dirfd, &path); + // the buffer argument of `readlink` and `readlinkat` has been removed, + // and the return value is now an owned type (`OsString`). + // Existing code can be updated by removing the buffer argument + // and removing any clone or similar operation on the output + + // old code `readlink(&path, &mut buf)` can be replaced with the following + readlink(&path); // this returns OsString + + // old code `readlinkat(dirfd, &path, &mut buf)` can be replaced with the following + readlinkat(dirfd, &path); // this returns OsString ``` ### Fixed diff --git a/src/fcntl.rs b/src/fcntl.rs index 1ef26271ab..08c339a912 100644 --- a/src/fcntl.rs +++ b/src/fcntl.rs @@ -188,10 +188,9 @@ fn wrap_readlink_result(v: &mut Vec, res: ssize_t) -> Result { } pub fn readlink<'a, P: ?Sized + NixPath>(path: &P) -> Result { - let len = libc::PATH_MAX as usize; - let mut v = Vec::with_capacity(len); + let mut v = Vec::with_capacity(libc::PATH_MAX as usize); let res = path.with_nix_path(|cstr| { - unsafe { libc::readlink(cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, len as size_t) } + unsafe { libc::readlink(cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.capacity() as size_t) } })?; wrap_readlink_result(&mut v, res) @@ -199,10 +198,9 @@ pub fn readlink<'a, P: ?Sized + NixPath>(path: &P) -> Result { pub fn readlinkat<'a, P: ?Sized + NixPath>(dirfd: RawFd, path: &P) -> Result { - let len = libc::PATH_MAX as usize; - let mut v = Vec::with_capacity(len); + let mut v = Vec::with_capacity(libc::PATH_MAX as usize); let res = path.with_nix_path(|cstr| { - unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, len as size_t) } + unsafe { libc::readlinkat(dirfd, cstr.as_ptr(), v.as_mut_ptr() as *mut c_char, v.capacity() as size_t) } })?; wrap_readlink_result(&mut v, res) From fa50f631979ca66ee80525c0140597fe7f704b9a Mon Sep 17 00:00:00 2001 From: Sendil Kumar N Date: Thu, 22 Aug 2019 20:34:53 +0000 Subject: [PATCH 8/8] Update CHANGELOG.md --- CHANGELOG.md | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2765c33349..3f89df4aab 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,18 +10,17 @@ This project adheres to [Semantic Versioning](http://semver.org/). ([#1109](https://github.com/nix-rust/nix/pull/1109)) ```rust - use nix::fcntl::{readlink, readlinkat}; - + # use nix::fcntl::{readlink, readlinkat}; // the buffer argument of `readlink` and `readlinkat` has been removed, // and the return value is now an owned type (`OsString`). // Existing code can be updated by removing the buffer argument // and removing any clone or similar operation on the output // old code `readlink(&path, &mut buf)` can be replaced with the following - readlink(&path); // this returns OsString + let _: OsString = readlink(&path); // old code `readlinkat(dirfd, &path, &mut buf)` can be replaced with the following - readlinkat(dirfd, &path); // this returns OsString + let _: OsString = readlinkat(dirfd, &path); ``` ### Fixed