improve install /dev/zero workaround

[btittelbach]

motivation

Just watched the FOSDEM 2023 talk where the workaround for install /dev/zero <target> is mentioned and shown on slide.

issue

This workaround, likely obvious to the original coder, not only breaks when used with other device files, but also if /dev/zero is named differently e.g. someone creates mknod ./zero2 c 1 3 and want to use install on it.

While that is of course, very unlikely, it still think it's bad to start making assumptions like this in the core utilities.

naive question

would it not be better to use two open, read, write instead and implement naive copying, rather than rely on a fs::copy that is broken from a unix PoV?

[sylvestre]

This is indeed a terrible workaround :)

Don't hesitate to provide a PR as better alternative

[meisterluk]

Hi @btittelbach, hi @sylvestre!

Certainly std::fs::canonicalize would be an improvement. Thus, I would recommend …

-    if from.as_os_str() == "/dev/null" {
-        /* workaround a limitation of fs::copy
-         * https://github.com/rust-lang/rust/issues/79390
-         */
-        if let Err(err) = File::create(to) {
-            return Err(
-                InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
-            );
+    let mut special_device_behavior = false;
+    if let Ok(filepath) = fs::canonicalize(from.as_os_str()) {
+        if filepath == PathBuf::from("/dev/null") {
+            special_device_behavior = true;
+            /* workaround a limitation of fs::copy
+             * https://github.com/rust-lang/rust/issues/79390
+             */
+            if let Err(err) = File::create(to) {
+                return Err(
+                    InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into(),
+                );
+            }
+        }
+    }
+
+    if !special_device_behavior {
+        if let Err(err) = fs::copy(from, to) {
+            return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
         }
-    } else if let Err(err) = fs::copy(from, to) {
-        return Err(InstallError::InstallFailed(from.to_path_buf(), to.to_path_buf(), err).into());
     }

… but writing a proper testcase is difficult. So if I run install /dev/zero xyz with coreutils, the copy operation never stops. In order to write a proper testcase, we need means to limit the amount of copied files finitely. Does anyone have an idea?