Rust version of my C++ command-line tool

Question

First program I've written in Rust (besides the demos in the tutorial site).

I decided to start by converting my C++ command line tool to Rust. This is not necessarily a 1 to 1 conversion, but I've tried to maintain the overall structure the same:

// Currently (rustc 1.0.0-nightly), these modules are all unstable
// and would generate warnings without these suppressions.
#![feature(io)]
#![feature(path)]
#![feature(collections)]

use std::fs::File;
use std::fs::Metadata;
use std::path::Path;
use std::io::prelude::*;

// ========================================================

// The GTA Vice City ADF files are MP3 files that
// had each byte XORed with this magic constant.
// 34 is 22 in hexadecimal and 42 in octal...
//
// Not sure who figured this out, but I got this
// info from the Xentax File Format Wiki.
//
static GTA_MAGIC : u8 = 34;

// Process the input file in chunks of this size in bytes.
// The chunk buffer is allocated dynamically (it is a Vec)
// so this can be a fairly large value.
//
static CHUNK_SIZE : usize = 8192;

// ========================================================

fn print_help_text(program_name : &String) {
    println!("");
    println!("Usage:");
    println!("$ {} <input_file> [output_file]", program_name);
    println!("  Runs the tool normally. If the output filename is not provided");
    println!("  the input filename is used but the extension is replaced with '.mp3'.");
    println!("");
    println!("Usage:");
    println!("$ {} --help | -h", program_name);
    println!("  Prints this help text.");
    println!("");
}

fn remove_extension(filename : &String) -> String {
    let path = Path::new(filename);
    return String::from_str(path.file_stem()
                .unwrap().to_str().unwrap());
}

fn open_file(filename : &String) -> (File, Metadata) {
    let path = Path::new(filename);

    // `open()` is read-only by default.
    let file_handle = match File::open(&path) {
        Err(why) => panic!("Couldn't open file {}: {}", path.display(), why.description()),
        Ok(file_handle) => file_handle,
    };

    let file_info = match file_handle.metadata() {
        Err(why) => panic!("Unable to get metadata for {}: {}", path.display(), why.description()),
        Ok(file_info) => file_info,
    };

    // Return a pair of file handle and its metadata:
    return (file_handle, file_info);
}

fn create_file(filename : &String) -> File {
    let path = Path::new(filename);

    // `create()` is write-only by default.
    return match File::create(&path) {
        Err(why) => panic!("Couldn't create file {}: {}", path.display(), why.description()),
        Ok(file_handle) => file_handle,
    };
}

fn process_files(input_filename : &String, output_filename : &String) {
    // Source file (ADF):
    let in_file_tuple = open_file(input_filename);
    let in_file_len   = in_file_tuple.1.len() as usize; // Index 1: Metadata
    let mut in_file   = in_file_tuple.0;                // Index 0: File

    // Destination file (MP3):
    let mut out_file = create_file(output_filename);

    // Chunk processing loop:
    let mut bytes_processed : usize = 0;
    while bytes_processed != in_file_len {
        let bytes_left = in_file_len - bytes_processed;
        let bytes_this_iteration = std::cmp::min(CHUNK_SIZE, bytes_left);

        let mut chunk = vec![0u8; bytes_this_iteration];

        match in_file.read(chunk.as_mut_slice()) {
            Err(why) => panic!("Failed to read input file! {}", why.description()),
            Ok(_) => {},
        };

        chunk = chunk.map_in_place(|byte| byte ^ GTA_MAGIC);

        match out_file.write(chunk.as_mut_slice()) {
            Err(why) => panic!("Failed to write output file! {}", why.description()),
            Ok(_) => {},
        };

        bytes_processed += bytes_this_iteration;
    }
}

fn main() {
    // Fetch command line:
    let cmd_args : Vec<String> =
            std::env::args()
                .map(|x| x.to_string())
                    .collect();

    // Too few command line args, exit:
    if cmd_args.len() < 2 {
        println!("Not enough arguments!");
        print_help_text(&cmd_args[0]);
        return;
    }

    // Print help and exit if "-h" or "--help" present in cmd line:
    if cmd_args.len() > 1 && (cmd_args[1] == "-h" || cmd_args[1] == "--help") {
        print_help_text(&cmd_args[0]);
        return;
    }

    //
    // Normal execution path:
    //
    let input_filename : String;
    let mut output_filename : String;

    if cmd_args.len() >= 3 {
        // input_filename + output_filename:
        input_filename  = cmd_args[1].clone();
        output_filename = cmd_args[2].clone();
    } else {
        // Just input_filename:
        input_filename  = cmd_args[1].clone();
        output_filename = String::new();
    }

    // Replace ".adf" extension of source filename with ".mp3" and use
    // it for the output if no explicit filename was provided.
    if output_filename.is_empty() {
        output_filename = remove_extension(&input_filename) + ".mp3";
    }

    process_files(&input_filename, &output_filename);
}

I'm looking for comments on style, design, nitpicking, anything to improve the code, really.

Specifically, the file processing loop seems to be somewhat inefficient and doing a lot of allocations/deallocations. I couldn't figure out how to read/write chunks of a file in a simpler way; read()/write() both infer the number of bytes to operate on from the input array, so I had to allocate a new one each iteration. Seems wasteful...

Edit note:

Initially I was not compiling the Rust code with proper optimizations enabled (see @Shepmaster's comment below), which produced very poor timings when compared with the C++ code (rookie's mistake, no doubt ;)). With -O added to the command line, it performed slightly faster than the C++ equivalent on my machine. Not bad at all!

Answer 1

Here's some feedback, in no interesting order:

// Current
static GTA_MAGIC : u8 = 34;
// Preferred 
const GTA_MAGIC: u8 = 34;

Two fixes here - Using const instead of static. static items exist in the compiled code, and can have their address taken. You don't need that here, so prefer const. Also, don't put spaces before the : when describing types.

// Current
fn print_help_text(program_name: &String) {
// Preferred
fn print_help_text(program_name: &str) {

Prefer to accept &str when you are just reading a string. This allows more things to be provided to the function, including static strings.

// Current
return (file_handle, file_info);
// Preferred
(file_handle, file_info)

Don't use explicit return statements at end of functions.

// Current
chunk = chunk.map_in_place(|byte| byte ^ GTA_MAGIC);
// Preferred
for byte in &mut chunk { *byte ^= GTA_MAGIC }

map (or map_in_place) isn't idiomatic when used just for side-effects. Just use a simple for loop.

match out_file.write(chunk.as_mut_slice()) {
    Ok(_) => {}, // OH NOES
};

write is not guaranteed to write all the bytes you asked for! Ignoring the result here means that you don't know how much was written, and may be losing data.

for byte in in_file.bytes() {
    let byte = byte.ok().unwrap();
    out_file.write(&[byte ^ GTA_MAGIC]).unwrap();
}

Instead of the whole buffering loop thing, try this simpler version. I punted on graceful error-handling. If you see performance decrease, I'd suggest wrapping in_file and out_file in BufferedReader / BufferedWriter instances.

let cmd_args: Vec<_> =
    std::env::args()
    .map(|x| x.to_string())
    .collect();

Method chains should be at the same level of indentation. You can also leave out String and let the compiler infer it for you.

// Current
let mut output_filename: String; 
// Preferred
let mut output_filename;         

Don't write out type names when you don't need to.