stty: set control characters

[willshuttleworth]

Added functionality to set mappings between control characters (erase, intr, kill, etc.) and specific characters given by the user (^C, for example).

I had to rework the argument processing as it was previously processing one flag at a time, but this feature requires processing of consecutive args (ie stty erase ^C). Added string_to_control_char() to convert strings like "^C", "X", "0x7F" to their ASCII value if they are valid control char mappings. Also added apply_char_mapping() to set new control char mappings using termios.

I'm a relatively new Rust programmer, so I'd love feedback on things that could be written more idiomatically or concisely. This PR is related to issue #7357.

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/stdbuf (fails in this run but passes in the 'main' branch)

[cakebaker]

As you only handle the error case, you can simplify it with map_err:

Suggested change

	match apply_char_mapping(&mut termios, char_index, new_cc) {
	Ok(_) => {}
	Err(e) => match e {
	ControlCharMappingError::IntOutOfRange => {
	return Err(USimpleError::new(
	1,
	format!(
	"invalid integer argument: '{new_cc}': Numerical result out of range"
	),
	));
	}
	ControlCharMappingError::MultipleChars => {
	return Err(USimpleError::new(
	1,
	format!("invalid integer argument: '{new_cc}'"),
	));
	}
	},
	}
	apply_char_mapping(&mut termios, char_index, new_cc).map_err(|e| {
	let message = match e {
	ControlCharMappingError::IntOutOfRange => format!(
	"invalid integer argument: '{new_cc}': Numerical result out of range"
	),
	ControlCharMappingError::MultipleChars => {
	format!("invalid integer argument: '{new_cc}'")
	}
	};
	USimpleError::new(1, message)
	})?;

[cakebaker]

This function name is a bit misleading: functions starting with is_ usually return a bool.

[willshuttleworth]

initially i was returning a bool but then changed it to an option, good catch

[cakebaker]

I think you can simplify it to something like:

Suggested change

	match string_to_control_char(new_val) {
	Ok(val) => {
	termios.control_chars[control_char_index as usize] = val;
	Ok(())
	}
	Err(e) => Err(e),
	}
	string_to_control_char(new_val).and_then(|val| {
	termios.control_chars[control_char_index as usize] = val;
	Ok(())
	})

[cakebaker]

The function returns a ControlCharMappingError in the error case, not None.

[cakebaker]

ascii_num doesn't have to be mutable. And you can get rid of the last if let:

Suggested change

	let mut ascii_num: Option<u32> = None;
	if let Some(hex) = s.strip_prefix("0x") {
	ascii_num = u32::from_str_radix(hex, 16).ok();
	} else if let Some(octal) = s.strip_prefix("0") {
	ascii_num = u32::from_str_radix(octal, 8).ok();
	} else if let Ok(decimal) = s.parse::<u32>() {
	ascii_num = Some(decimal);
	}
	let ascii_num = if let Some(hex) = s.strip_prefix("0x") {
	u32::from_str_radix(hex, 16).ok()
	} else if let Some(octal) = s.strip_prefix("0") {
	u32::from_str_radix(octal, 8).ok()
	} else {
	s.parse::<u32>().ok()
	};

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/misc/usage_vs_getopt (fails in this run but passes in the 'main' branch)
Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

[willshuttleworth]

anything else you'd like me to fix? i can push the final review commit now if we're all good

[cakebaker]

@willshuttleworth from my side it looks good, maybe @tertsdiepraam has something to add as he wrote most of the current implementation.

[willshuttleworth]

@cakebaker i have committed the final code review related changes

[github-actions]

GNU testsuite comparison:

Skip an intermittent issue tests/timeout/timeout (fails in this run but passes in the 'main' branch)

[sylvestre]

it would be nice to have tests

[willshuttleworth]

the one issue with testing is that cargo test doesnt run in a tty, so i would need to do some reworking to test properly. there are two main options i can imagine now. this first is reworking uumain() so that control char parsing is done before the call to tcgetattr() and i can test for argument combinations that fail. or, i could rework the way testing is done so that a separate tty process is spawned and used. this crate seems to accomplish that. the first option seems to be the easier and more pragmatic approach, but i could be ignorant of better options.

[sylvestre]

that would be ideal
you could also start by testing at function level (like cc_to_index)