Pseudo-random number generator & password generator

Question

I've started learning Rust and decided to implement something on my own from scratch. I've implemented a PRNG and I use it to generate random passwords.

Project tree:

| .gitignore
| Cargo.lock
| Cargo.toml
|
+---src
| main.rs
| password.rs
| xorshift.rs
|

main.rs :

use std::io;
mod xorshift;
mod password;
fn main() {
 let length : u32;
 loop{
 let mut pass_leng_str = String::new();
 println!("Enter the length of wannable password: ");
 io::stdin()
 .read_line(&mut pass_leng_str)
 .expect("Faild to read the line");
 let pass_leng_str = pass_leng_str.trim();
 match pass_leng_str.parse::<u32>(){
 Ok(i) => {
 length = i;
 break;
 },
 Err(..) => {
 println!("Not a valid integer!"); 
 },
 }
 }
 println!("Enter the characters you want to exclude: ");
 let mut exclude = String::new();
 io::stdin()
 .read_line(&mut exclude)
 .expect("Faild to read the line");
 
 let excluded : Vec<char> = exclude.chars().collect();
 println!("{}", password::rand_pass(length, excluded));
}

password.rs :

use super::xorshift;
pub static ASCII : [char; 69] = [
 'a', 'b', 'c', 'd', 'e', 
 'f', 'g', 'h', 'i', 'j', 
 'k', 'l', 'm', 'n', 'o',
 'p', 'q', 'r', 's', 't', 
 'u', 'v', 'w', 'x', 'y', 
 'z', 
 'A', 'B', 'C', 'D', 'E',
 'F', 'G', 'H', 'I', 'J',
 'K', 'L', 'M', 'N', 'O',
 'P', 'Q', 'R', 'S', 'T',
 'U', 'V', 'W', 'X', 'Y',
 'Z',
 '1','2','3','4',
 '5','6','7','8',
 '9','0',
 '+','-','_','?','!','$','/',
];
pub fn rand_pass(pass_len : u32, excluded : Vec<char>)-> String{
 let mut password = String::new();
 for _i in 0..pass_len{
 let c : char = loop{
 let c = ASCII[xorshift::get_rand(69) as usize];
 if !excluded.contains(&c){
 break c
 }
 };
 password.push(c);
 }
 password
}

xorshift.rs :

use std::time::UNIX_EPOCH;
use std::time::SystemTime;
pub fn get_rand(len : u128) -> u128{
 let now = SystemTime::now();
 let from_unix = now.duration_since(UNIX_EPOCH).expect("Congrats on time travel!");
 let seed = from_unix.as_nanos();
 let x = seed;
 let x = x ^ seed << 13;
 let x = x ^ x >>17;
 let x = x ^ x <<5;
 let x = x % len;
 x as u128
}

Answer 1

Welcome to Rust, and welcome to Code Review!

rustfmt & clippy

Always run cargo fmt and cargo clippy first. cargo fmt formats your code according to the official Rust Style Guide, and cargo clippy detects common mistakes and provides feedback on improving your code (none in this case).

main.rs

The biggest problem with the main function is its structure. Logically, the job of main is to:

• read the length of the password;

• read the excluded characters; and

• output the generated password.

You spent so much code on the first step that it is hard to recognize at a glance without being familiar with input loops. For readability, it is preferable to extract all of this into a function:

fn input_length() -> usize {
 loop {
 eprintln!("Enter the length of password: ");
 let mut length = String::new();
 io::stdin()
 .read_line(&mut length)
 .expect("cannot read line");
 match length.trim().parse() {
 Ok(length) => return length,
 Err(_) => eprintln!("Error: invalid length\n"),
 }
 }
}

Lengths are usually represented by usize in Rust to avoid type casts, instead of u32.

It is recommended to define variables as close to their usage as possible.

Personally, I would change all occurrences of println! to eprintln! expect the one that actually prints the password, so that the program can be easily used in an automated script by reading stdout.

Moving on, we see that there is no need to extract all characters in exclude, a String, into excluded, a Vec<char> (the naming can be improved, by the way). A String encodes the characters in UTF-8, which is variable-length, so ASCII characters take up only 1 byte each, whereas a sequence of chars always stores each character as 4 bytes. Moreover, UTF-8 is specially constructed so that searching requires only a linear scan and no computation of character boundaries, so there is no performance gain.

password.rs

It shouldn't be the job of the password module to expose a constant array containing ASCII characters. A better name for ASCII may be ALLOWED_CHARS.

The function rand_pass will often be called with the password:: prefix, so it is not necessary to repeat this information. I would name it generate.

Since the function does not consume excluded, take a &[char], or better, &str parameter. See Why is it discouraged to accept a reference to a String (&String), Vec (&Vec), or Box (&Box) as a function argument?.

The function can be rewritten in a more logically straightforward manner using iterators:

use std::iter;
pub fn generate(length: usize, excluded: &str) -> String {
 iter::repeat_with(generate_char)
 .filter(|c| !excluded.contains(&c))
 .take(length)
 .collect()
}

where generate_char is a function Fn() -> char that generates an individual random character in the password, omitted for simplicity. The intent is now clearer.

xorshift.rs

A better name for the module might be time_rng.

use declarations can be condensed:

use std::time::{SystemTime, UNIX_EPOCH};

A mutable variable is more readable to me here:

let mut x = seed;
x ^= (seed << 13);
x ^= (x >> 17);
x ^= (x << 5);
x % len

It is best to not count on the reader knowing the operator precedence here.