Merge #1242

1242: Don't implement `NixPath` for `Option<&P> where P: NixPath` r=asomers a=asomers

Most Nix functions that accept `NixPath` arguments can't do anything
useful with `None`.  The exceptions (`mount` and `quotactl_sync`)
already take explicitly optional arguments.

Also, this changes the behavior of `mount` with `None` arguments.
Previously, it would call mount(2) with empty strings for those
arguments.  Now, it will use null pointers.

Co-authored-by: Alan Somers <asomers@gmail.com>

CHANGELOG.md

@@ -57,6 +57,12 @@ This project adheres to [Semantic Versioning](http://semver.org/).
 - Removed `sys::socket::addr::from_libc_sockaddr` from the public API.
   (#[1215](https://github.com/nix-rust/nix/pull/1215))
 
+- Nix no longer implements `NixPath` for `Option<P> where P: NixPath`.  Most
+  Nix functions that accept `NixPath` arguments can't do anything useful with
+  `None`.  The exceptions (`mount` and `quotactl_sync`) already take explicitly
+  optional arguments.
+  (#[1242](https://github.com/nix-rust/nix/pull/1242))
+
 ## [0.17.0] - 3 February 2020
 ### Added
 - Add `CLK_TCK` to `SysconfVar`

src/lib.rs

@@ -283,22 +283,3 @@ impl NixPath for PathBuf {
         self.as_os_str().with_nix_path(f)
     }
 }
-
-/// Treats `None` as an empty string.
-impl<'a, NP: ?Sized + NixPath>  NixPath for Option<&'a NP> {
-    fn is_empty(&self) -> bool {
-        self.map_or(true, NixPath::is_empty)
-    }
-
-    fn len(&self) -> usize {
-        self.map_or(0, NixPath::len)
-    }
-
-    fn with_nix_path<T, F>(&self, f: F) -> Result<T> where F: FnOnce(&CStr) -> T {
-        if let Some(nix_path) = *self {
-            nix_path.with_nix_path(f)
-        } else {
-            unsafe { CStr::from_ptr("\0".as_ptr() as *const _).with_nix_path(f) }
-        }
-    }
-}

src/mount.rs

@@ -61,22 +61,33 @@ pub fn mount<P1: ?Sized + NixPath, P2: ?Sized + NixPath, P3: ?Sized + NixPath, P
         flags: MsFlags,
         data: Option<&P4>) -> Result<()> {
 
-    let res =
-        source.with_nix_path(|source| {
-            target.with_nix_path(|target| {
-                fstype.with_nix_path(|fstype| {
-                    data.with_nix_path(|data| {
-                        unsafe {
-                            libc::mount(source.as_ptr(),
-                                       target.as_ptr(),
-                                       fstype.as_ptr(),
-                                       flags.bits,
-                                       data.as_ptr() as *const libc::c_void)
-                        }
-                    })
+    fn with_opt_nix_path<P, T, F>(p: Option<&P>, f: F) -> Result<T>
+        where P: ?Sized + NixPath,
+              F: FnOnce(*const libc::c_char) -> T
+    {
+        match p {
+            Some(path) => path.with_nix_path(|p_str| f(p_str.as_ptr())),
+            None => Ok(f(std::ptr::null()))
+        }
+    }
+
+    let res = with_opt_nix_path(source, |s| {
+        target.with_nix_path(|t| {
+            with_opt_nix_path(fstype, |ty| {
+                with_opt_nix_path(data, |d| {
+                    unsafe {
+                        libc::mount(
+                            s,
+                            t.as_ptr(),
+                            ty,
+                            flags.bits,
+                            d as *const libc::c_void
+                        )
+                    }
                 })
             })
-        })????;
+        })
+    })????;
 
     Errno::result(res).map(drop)
 }

src/sys/quota.rs

@@ -253,6 +253,8 @@ pub fn quotactl_off<P: ?Sized + NixPath>(which: QuotaType, special: &P) -> Resul
 }
 
 /// Update the on-disk copy of quota usages for a filesystem.
+///
+/// If `special` is `None`, then all file systems with active quotas are sync'd.
 pub fn quotactl_sync<P: ?Sized + NixPath>(which: QuotaType, special: Option<&P>) -> Result<()> {
     quotactl(QuotaCmd(QuotaSubCmd::Q_SYNC, which), special, 0, ptr::null_mut())
 }