Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 35 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
@@ -1 +1,35 @@
# minishell
# [minishell](https://gh.jschwabe.site/42_minishell)-rs

A gradual rewrite of a [c2rust](https://github.com/immunant/c2rust) transpiled [codebase](https://gh.jschwabe.site/42_minishell).

Discover how C language programming constructs can be implemented in a more concise way.

## goals
1. see where rust syntax and std containers can enable better readability or code structure
2. discover where [`CString`](https://doc.rust-lang.org/stable/std/ffi/struct.CString.html), [`CStr`](https://doc.rust-lang.org/stable/std/ffi/struct.CStr.html) and `&[u8]` ([u8](https://doc.rust-lang.org/stable/std/primitive.u8.html)) can find usage and reduce interactions with `char *`
3. replace `extern "C"` and [libc](https://docs.rs/libc/latest/libc/) usage with [nix](https://docs.rs/nix/latest/nix/) wrappers, enabling more idiomatic (and less error prone?) usage of unix system functions.
4. provide me with an excuse to write rust code
## journey
### context

The original implementation used loads of custom glue that could have been replaced with libc functions (strtok, strcoll, scanf/sprintf, fprintf).

This was fine as a school project and provided me with ample opportunities for refactoring in the rust version.

### key takeaways
- since rust is not c, interacting with raw pointers is more error prone due to its memory model and my assumptions about memory derived from c
- signal handling was easier to do in c, I ended up removing it as it caused weird bugs
- even though rust has [`Command`](https://doc.rust-lang.org/std/process/struct.Command.html), it was not used. I adapted my execution logic to be more idiomatic by using nix wrappers (the reason for most [`unsafe`](https://doc.rust-lang.org/std/keyword.unsafe.html) usage)
- taking advantage of rust's rich type system can improve readability and facilitate quick iteration - [choosing the correct tool is key](https://doc.rust-lang.org/std/)

## process
### tactics
1. transpile, then simplify some operations, mostly aligning types (e.g. [`libc::size_t`](https://docs.rs/libc/latest/libc/type.size_t.html) for [`u64`](https://doc.rust-lang.org/core/primitive.u64.html)/[`usize`](https://doc.rust-lang.org/core/primitive.usize.html)), removing non-needed casts and replacing [`.offset()`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.offset) with [`.add()`](https://doc.rust-lang.org/std/ptr/struct.NonNull.html#method.add)
2. multiple failed rewrites of core functionality caused by newly introduced logic bugs - mitigated by comprehensive [test cases](https://docs.rs/rstest/latest/rstest/attr.rstest.html#test-parametrized-cases)
3. issues with readability, naming - resolved by using more idiomatic constructs: [`Option`](https://doc.rust-lang.org/std/option/enum.Option.html), [`Result`](https://doc.rust-lang.org/std/result/enum.Result.html), [tuples](https://doc.rust-lang.org/std/primitive.tuple.html), [slices](https://doc.rust-lang.org/std/primitive.slice.html) instead of [references](https://doc.rust-lang.org/std/primitive.reference.html), [`impl`](https://doc.rust-lang.org/std/keyword.impl.html) for structs
4. remove duplicate or dead code replaced by std ([`format!`](https://doc.rust-lang.org/std/macro.format.html), [`vec![]`](https://doc.rust-lang.org/std/macro.vec.html), ...)

### strategy
1. document & simplify original logic
2. create adaptable tests, wrappers fitting both c-like and idiomatic rust outputs
3. apply TDD feedback loop while simplifying logic and reducing [`unsafe`](https://doc.rust-lang.org/std/keyword.unsafe.html)
4 changes: 2 additions & 2 deletions src/execution/execute_pipes.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ unsafe fn exec_last(shell: &mut t_shell, i: usize, prevpipe: *mut i32) {
_ => (),
},
Ok(ForkResult::Child) => {
// check signals child
// previously: check signals child
if shell.token_vec[i].has_redir {
do_heredocs(&shell.token_vec[i], &mut *prevpipe, &shell.env);
}
Expand All @@ -47,7 +47,7 @@ unsafe fn exec_pipe(shell: &mut t_shell, i: usize, prevpipe: *mut i32) {
*prevpipe = pipefd[0_usize];
}
Ok(ForkResult::Child) => {
// check signals child
// previously: check signals child
libc::close(pipefd[0_usize]);
libc::dup2(pipefd[1_usize], 1);
libc::close(pipefd[1_usize]);
Expand Down
7 changes: 1 addition & 6 deletions src/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,6 @@

extern crate libc;

// to spend more time ;\
// handle signals? removed - non-functional - partially working in rustyline
// use tempfile for heredoc (if possible!)

mod environment;
mod execution;
mod lexer;
Expand All @@ -18,7 +14,7 @@ use rustyline::{DefaultEditor, Result, error::ReadlineError};

pub fn main() -> Result<()> {
let mut shell = t_shell::new();
// check signals
// previously: check signals
let mut rl = DefaultEditor::new()?;
loop {
let readline = rl.readline("minishell> ");
Expand All @@ -35,7 +31,6 @@ pub fn main() -> Result<()> {
} else if shell.tokenize(trimmed_line).is_none() {
continue;
} else {
// dbg!(&shell.token_vec);
crate::execution::execute_commands(&mut shell);
}
}
Expand Down
4 changes: 2 additions & 2 deletions src/msh.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,8 +39,8 @@ pub(crate) struct t_token {
#[derive(Clone, Debug, PartialEq)]
pub(crate) struct t_arg {
pub elem_str: String,
pub type_0: e_arg, // wrapped enum attribute
pub redir: Option<e_redir>, // enum wrapping string
pub type_0: e_arg,
pub redir: Option<e_redir>,
}

impl t_arg {
Expand Down
Loading