From 7069448050e4f9db837ab7c99f8f1bef594f2077 Mon Sep 17 00:00:00 2001 From: nibon7 Date: Fri, 26 Jan 2024 01:53:18 +0800 Subject: [PATCH] Don't panic when creating `FileBackedHistory` with `usize::MAX` capacity (#701) --- examples/demo.rs | 2 +- src/history/base.rs | 10 +++++++++- src/history/file_backed.rs | 27 +++++++++++++++++---------- src/result.rs | 6 ++++++ 4 files changed, 33 insertions(+), 12 deletions(-) diff --git a/examples/demo.rs b/examples/demo.rs index 2e7a402e..f1b6d576 100644 --- a/examples/demo.rs +++ b/examples/demo.rs @@ -18,7 +18,7 @@ use reedline::CursorConfig; #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))] use reedline::FileBackedHistory; -fn main() -> std::io::Result<()> { +fn main() -> reedline::Result<()> { println!("Ctrl-D to quit"); // quick command like parameter handling let vi_mode = matches!(std::env::args().nth(1), Some(x) if x == "--vi"); diff --git a/src/history/base.rs b/src/history/base.rs index 2fcbe4ac..4c07417b 100644 --- a/src/history/base.rs +++ b/src/history/base.rs @@ -431,7 +431,7 @@ mod test { #[cfg(not(any(feature = "sqlite", feature = "sqlite-dynlib")))] #[test] fn history_size_zero() -> Result<()> { - let mut history = crate::FileBackedHistory::new(0); + let mut history = crate::FileBackedHistory::new(0)?; history.save(create_item(1, "/home/me", "cd ~/Downloads", 0))?; assert_eq!(history.count_all()?, 0); let _ = history.sync(); @@ -440,4 +440,12 @@ mod test { Ok(()) } + + #[test] + fn create_file_backed_history() { + use crate::HISTORY_SIZE; + + assert!(crate::FileBackedHistory::new(usize::MAX).is_err()); + assert!(crate::FileBackedHistory::new(HISTORY_SIZE).is_ok()); + } } diff --git a/src/history/file_backed.rs b/src/history/file_backed.rs index ec70501f..ffff840b 100644 --- a/src/history/file_backed.rs +++ b/src/history/file_backed.rs @@ -37,8 +37,15 @@ impl Default for FileBackedHistory { /// Creates an in-memory [`History`] with a maximal capacity of [`HISTORY_SIZE`]. /// /// To create a [`History`] that is synchronized with a file use [`FileBackedHistory::with_file()`] + /// + /// # Panics + /// + /// If `HISTORY_SIZE == usize::MAX` fn default() -> Self { - Self::new(HISTORY_SIZE) + match Self::new(HISTORY_SIZE) { + Ok(history) => history, + Err(e) => panic!("{}", e), + } } } @@ -286,20 +293,20 @@ impl History for FileBackedHistory { impl FileBackedHistory { /// Creates a new in-memory history that remembers `n <= capacity` elements /// - /// # Panics - /// - /// If `capacity == usize::MAX` - pub fn new(capacity: usize) -> Self { + pub fn new(capacity: usize) -> Result { if capacity == usize::MAX { - panic!("History capacity too large to be addressed safely"); + return Err(ReedlineError(ReedlineErrorVariants::OtherHistoryError( + "History capacity too large to be addressed safely", + ))); } - FileBackedHistory { + + Ok(FileBackedHistory { capacity, entries: VecDeque::new(), file: None, len_on_disk: 0, session: None, - } + }) } /// Creates a new history with an associated history file. @@ -310,8 +317,8 @@ impl FileBackedHistory { /// /// **Side effects:** creates all nested directories to the file /// - pub fn with_file(capacity: usize, file: PathBuf) -> std::io::Result { - let mut hist = Self::new(capacity); + pub fn with_file(capacity: usize, file: PathBuf) -> Result { + let mut hist = Self::new(capacity)?; if let Some(base_dir) = file.parent() { std::fs::create_dir_all(base_dir)?; } diff --git a/src/result.rs b/src/result.rs index e4c7344b..efe98795 100644 --- a/src/result.rs +++ b/src/result.rs @@ -33,6 +33,12 @@ pub enum ReedlineErrorVariants { #[derive(Debug)] pub struct ReedlineError(pub ReedlineErrorVariants); +impl From for ReedlineError { + fn from(err: std::io::Error) -> Self { + Self(ReedlineErrorVariants::IOError(err)) + } +} + impl Display for ReedlineError { fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { self.0.fmt(f)