Merge #1109

1109: fix readlink/readlinkat to return too long only when it is long r=asomers a=sendilkumarn

Currently readlink returns `ENAMETOOLONG` when the bufferSize is equal to the result size.  

Consider the below case

```c++
int main(int argc, const char * argv[]) {
    
    std::string filename = "/tmp/test-dir/target";
    
    size_t bufferSize = 9;
    char* buffer = new char[bufferSize];
    size_t rc = readlink (filename.c_str(), buffer, bufferSize); 
   // Since `readlink's` output depends on the `bufferSize`. If the`bufferSize` is 9 or greater than 
   // 9 then it will return 9 or else it will return `bufferSize` as the value of `rc`. 

    return 0;
}
```

Co-authored-by: Sendil Kumar <[email protected]>
Co-authored-by: Sendil Kumar N <[email protected]>

Author: bors[bot]

CHANGELOG.md

@@ -6,6 +6,23 @@ 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))
+
+  ```rust
+  # 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
+  let _: OsString = readlink(&path);
+  
+  // old code `readlinkat(dirfd, &path, &mut buf)` can be replaced with the following
+  let _: OsString = readlinkat(dirfd, &path);
+  ```
+
 ### Fixed
 ### Removed
 

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,34 +177,33 @@ pub fn renameat<P1: ?Sized + NixPath, P2: ?Sized + NixPath>(old_dirfd: Option<Ra
     Errno::result(res).map(drop)
 }
 
-fn wrap_readlink_result(buffer: &mut[u8], res: ssize_t) -> Result<&OsStr> {
+fn wrap_readlink_result(v: &mut Vec<u8>, res: ssize_t) -> Result<OsString> {
     match Errno::result(res) {
         Err(err) => Err(err),
         Ok(len) => {
-            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<OsString> {
+    let mut v = Vec::with_capacity(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.capacity() 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<OsString> {
+    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(), 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.capacity() 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`.

test/test_fcntl.rs

@@ -56,12 +56,11 @@ fn test_readlink() {
     let dirfd = open(tempdir.path(),
                      OFlag::empty(),
                      Mode::empty()).unwrap();
+    let expected_dir = src.to_str().unwrap();
+
+    assert_eq!(readlink(&dst).unwrap().to_str().unwrap(), expected_dir);
+    assert_eq!(readlinkat(dirfd, "b").unwrap().to_str().unwrap(), expected_dir);
 
-    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());
 }
 
 #[cfg(any(target_os = "linux", target_os = "android"))]

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(),