Merge pull request #11 from Jonstep101010/writeup

add documentation

Author: Jonstep101010

README.md

@@ -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)

src/execution/execute_pipes.rs

@@ -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);
 			}
@@ -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]);

src/main.rs

@@ -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;
@@ -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> ");
@@ -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);
 				}
 			}

src/msh.rs

@@ -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 {