HiveBrain v1.2.0
Get Started
← Back to all entries
patternrustModerate

Simple cat in Rust

Submitted by: @import:stackexchange-codereview··
0
Viewed 0 times
rustsimplecat

Problem

I'm learning Rust, from a Python background, and while I've used languages like C and C++ in the (distant) past, system languages aren't really my specialty.

I would just like to know if my code is sane, if I seem to have the principles roughly right, but I'd be happy to hear any improvements at all, particularly with performance. I'm also a bit unsure about error handling.

Anyway, for my first program I've made a simple implementation of the UNIX cat command, without support for any arguments:

use std::env;
use std::io;
use std::io::Read;
use std::io::Write;
use std::fs::File;

macro_rules! println_stderr(
    ($($arg:tt)*) => (
        match writeln!(&mut io::stderr(), $($arg)* ) {
            Ok(_) => {},
            Err(err) => panic!("Unable to write to stderr: {}", err),
        }
    )
);

fn main() {
    let args: Vec = env::args().collect();
    if args.len()  {},
                Err(err) => {
                    println_stderr!("{}", err.to_string());
                    continue;
                }
            };
            print!("{}", s);
        }
    } else {
        for arg in &args[1..] {
            let mut s = String::new();
            let mut file = match File::open(arg) {
                Ok(file) => file,
                Err(err) => {
                    println_stderr!("{}", err.to_string());
                    continue;
                }
            };

            match file.read_to_string(&mut s) {
                Ok(_) => {},
                Err(err) => {
                    println_stderr!("{}", err.to_string());
                    continue;
                }
            };

            print!("{}", s);
        }
    }
}

Solution

imports

Your imports should be compressed:

use std::io::{self, Read, Write};


Although it might be better to use the io prelude:

use std::io;
use std::io::prelude::*;


I prefer the former for its explicitness.

match vs if let

match io::stdin().read_line(&mut s) {
    Ok(_) => {},
    Err(err) => {
        println_stderr!("{}", err.to_string());
        continue;
    }
};


is better as just

if let Err(err) = io::stdin().read_line(&mut s) {
    println_stderr!("{}", err);
    continue;
};


The same occurs later, and in the macro.

prefer late initialization

You should only initialize s just before use in the second branch.

copy-paste error?

Your continue when reading from stdin is a bit optimistic. I'd suggest just exiting at that point - if reading one line fails then reading the rest probably will too.

plz no Unicode

You read and write strings holding the whole file (or line) - cat should work instead on raw bytes, and preferrably in large chunks too. The most obvious way is something like

fn redirect_stream(reader: &mut R, writer: &mut W, buffer: &mut Vec) -> io::Result
    where R: Read, W: Write
{
    let mut buffer = vec![0; 64 * 1024];

    loop {
        let len_read = try!(reader.read(&mut buffer));

        if len_read == 0 {
            return Ok(())
        }

        try!(writer.write_all(&buffer[..len_read]));
    }
}


plz no allocate

Using the above means we're stuck allocating 64k for each stream, even if there are many streams or the stream is line-buffered.

The first can be solved by passing the buffer into the function. The later can be solved by resizing the buffer up to some hard limit from a small size.

DRY

Note that there's still duplication between the args.len() < 2 and else branches. One could solve this by writing a wrapping function to extract this functionality.

fn handle_arg(reader: &mut R, writer: &mut W, buffer: &mut Vec)
    where R: Read, W: Write
{
    if let Err(err) = redirect_stream(reader, writer, buffer) {
        println_stderr!("{}", err.to_string());
    }
}


Making handle_arg a closure would be prettier but require dynamic dispatch. That's probably fine, but maybe less idiomatic:

let stdout = &mut io::stdout();
let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
let mut handle_arg = move |mut reader: &mut Read| {
    if let Err(err) = redirect_stream(&mut reader, stdout, buffer) {
        println_stderr!("{}", err.to_string());
    }
};


- means stdin?

This is more reasonable if we want to allow the code to accept - to mean standard input, as we can do

let mut args: Vec = env::args().skip(1).collect();
if args.is_empty() {
    args.push("-".into());
}

let stdout = &mut io::stdout();
let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
for arg in args {
    if arg == "-" {
        handle_arg(&mut io::stdin(), stdout, buffer);
        continue;
    }

    match File::open(arg) {
        Ok(ref mut file) => {
            handle_arg(file, stdout, buffer)
        },
        Err(err) => {
            println_stderr!("{}", err);
            continue;
        }
    }
}


Result

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

const SMALL_BUFFER_SIZE: usize = 256;
const LARGE_BUFFER_SIZE: usize = 64 * 1024;

macro_rules! println_stderr(
    ($($arg:tt)*) => (
        if let Err(err) = writeln!(&mut io::stderr(), $($arg)* ) {
            panic!("Unable to write to stderr: {}", err);
        }
    )
);

fn redirect_stream(reader: &mut R, writer: &mut W, buffer: &mut Vec) -> io::Result
    where R: Read, W: Write
{
    loop {
        let len_read = try!(reader.read(buffer));

        if len_read == 0 {
            return Ok(())
        }

        try!(writer.write_all(&buffer[..len_read]));

        if len_read == buffer.len() && len_read  = env::args().skip(1).collect();
    if args.is_empty() {
        args.push("-".into());
    }

    fn handle_arg(reader: &mut R, writer: &mut W, buffer: &mut Vec)
        where R: Read, W: Write
    {
        if let Err(err) = redirect_stream(reader, writer, buffer) {
            println_stderr!("{}", err.to_string());
        }
    }

    let stdout = &mut io::stdout();
    let buffer = &mut vec![0; SMALL_BUFFER_SIZE];
    for arg in args {
        if arg == "-" {
            handle_arg(&mut io::stdin(), stdout, buffer);
            continue;
        }

        match File::open(arg) {
            Ok(ref mut file) => {
                handle_arg(file, stdout, buffer)
            },
            Err(err) => {
                println_stderr!("{}", err);
                continue;
            }
        }
    }
}

Code Snippets

use std::io::{self, Read, Write};
use std::io;
use std::io::prelude::*;
match io::stdin().read_line(&mut s) {
    Ok(_) => {},
    Err(err) => {
        println_stderr!("{}", err.to_string());
        continue;
    }
};
if let Err(err) = io::stdin().read_line(&mut s) {
    println_stderr!("{}", err);
    continue;
};
fn redirect_stream<R, W>(reader: &mut R, writer: &mut W, buffer: &mut Vec<u8>) -> io::Result<()>
    where R: Read, W: Write
{
    let mut buffer = vec![0; 64 * 1024];

    loop {
        let len_read = try!(reader.read(&mut buffer));

        if len_read == 0 {
            return Ok(())
        }

        try!(writer.write_all(&buffer[..len_read]));
    }
}

Context

StackExchange Code Review Q#94941, answer score: 16

Revisions (0)

No revisions yet.