Implementation of UNIX `tee` in Rust

Question

I am currently reading Michael Kerrisk's book, The Linux Programming Interface. This is one of the exercises from the end of chapter 4 (I'm working in Rust, not C).

A Rust implementation of the tee program on UNIX systems. According to it's man page

The tee utility copies standard input to standard output, making a copy in zero or more files. The output is unbuffered.

So running echo "foo" | tee will print "foo" to the terminal. running echo "foo" | tee myfile will print "foo" to the terminal and also create the file myfile with the contents "foo".

It's a small example but I'd love feedback on whether this is idiomatic Rust code and whether the efficiency could be improved.

use std::env;
use std::io::{self, Read};
use std::io::prelude::Write;
use std::error::Error;
use std::fs::File;

fn main() {
    let fname = match env::args().nth(1) {
        Some(name) => name,
        None => String::from("/dev/null"),
    };

    let mut outfile = match File::create(&fname) {
        Ok(file) => file,
        Err(why) => panic!("{}", why.description()),
    };

    loop {
        let mut line = String::new();
        match io::stdin().read_to_string(&mut line) {
            Ok(_) => {}
            Err(_) => std::process::exit(0),
        }

        if line.is_empty() {
            std::process::exit(0);
        }

        println!("{}", &line);

        let _ = outfile.write(line.as_bytes());
    }
}

Answer 1

High-level issues:

1. making a copy in zero or more files

   It appears the code only handles zero or one file.

2. The output is unbuffered.

   The code is very much buffering the input. Check out read_to_string:

   Read all bytes until EOF in this source, placing them into buf.

   That means that the program has to wait for the entire input string to exist before it writes a single byte. That input could be gigabytes!

3. The code assumes that all input is a UTF-8 encoded string. This cat cannot be used with images or zipfiles any any kind of binary data.

---

More tactical issues:

1. Don't use single items from a prelude. The point of a prelude is to be a glob import of all the things you might want.

2. Check out all the methods that exist on Option or Result. For example, unwrap_or_else encapsulates the default value case.

3. Hardcoding /dev/null ties the implementation to a specific OS (maybe OK because you are making cat) but also to a filesystem layout. This cat can't be used when /dev/ isn't available, which would likely be during a nasty system emergency. Would be better for the program to not write anything instead of writing to /dev/null.

4. Printing out the errors description shows less information than printing out the error:

   other os error

   vs.

   Is a directory (os error 21)

5. expect may be more compact than a match or unwrap_or_else, depending on the desired error format.

6. I don't believe the loop will ever loop, since the entire input is read into the string as discussed above.

7. If the loop did loop, reallocating the entire string for each iteration is inefficient. Better to just clear it and reuse the memory.

8. There's no need to pass a reference when using println!; that macro automatically references all the arguments.

9. Don't ignore errors! let _ is a really big red flag. cat has few responsibilities, but writing output correctly is fairly critical! The program should exit when writing fails, such as when a pipe is closed.

10. The exit code isn't well defined in all cases. What is it when panicking?

11. Standard in and out would be repeatedly locked in each iteration. Not as efficient as a single lock.

use std::env;
use std::io::{self, Read, Write};
use std::fs::File;

fn main() {
    let fname = env::args().nth(1).unwrap_or_else(|| String::from("/dev/null"));

    let mut outfile = File::create(&fname).expect("Unable to create file");
    let mut line = String::new();

    loop {
        line.clear();

        io::stdin().read_to_string(&mut line).unwrap_or_else(|_| {
            std::process::exit(0)
        });

        if line.is_empty() {
            std::process::exit(0);
        }

        println!("{}", line);

        outfile.write(line.as_bytes()).expect("Unable to write!");
    }
}

---

Sorry for going a bit off the deep end, but the problem was interesting. I'd probably write it a little like this:

use std::env;
use std::io::{self, Read, Write};
use std::fs::File;

const BUFFER_SIZE: usize = 8 * 1024;

fn main() {
    let mut buf = vec![0; BUFFER_SIZE];

    let stdin = io::stdin();
    let mut stdin = stdin.lock();

    let stdout = io::stdout();
    let stdout = stdout.lock();

    let mut outputs: Vec<_> = env::args().skip(1).map(|fname| {
        let f = File::create(&fname).unwrap_or_else(|e| {
            panic!("Unable to create file {}: {}", fname, e);
        });

        Box::new(f) as Box<Write>
    }).collect();

    outputs.push(Box::new(stdout));

    loop {
        let bytes = stdin.read(&mut buf).expect("Unable to read input");

        if bytes == 0 { break }

        // **Very important**, otherwise you can end up with
        // Heartbleed-esque bugs! I'm chosing to shadow `buf` to
        // deliberately prevent using it again in this loop.
        let buf = &buf[..bytes];

        for output in &mut outputs {
            output.write_all(buf).expect("Unable to write output");
        }
    }
}