Skip to content

Commit

Permalink
Use u8 for internal verbosity level calculation
Browse files Browse the repository at this point in the history
This avoids integer overflow / underflow errors when calculating the
verbosity level. Prior to this change, constructing a `Verbosity` with
either parameter equal to `u8::MAX` would result in incorrect verbosity
level calculation.
  • Loading branch information
joshka committed Sep 26, 2024
1 parent 7f435cd commit a7f7bb2
Showing 1 changed file with 24 additions and 52 deletions.
76 changes: 24 additions & 52 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,30 +126,32 @@ impl<L: LogLevel> Verbosity<L> {
self.log_level().is_none()
}

fn verbosity(&self) -> i8 {
level_value(L::default()) - (self.quiet as i8) + (self.verbose as i8)
fn verbosity(&self) -> u8 {
let default_verbosity = level_value(L::default());
let verbosity = default_verbosity as i16 - self.quiet as i16 + self.verbose as i16;
verbosity.clamp(0, u8::MAX as i16) as u8
}
}

fn level_value(level: Option<Level>) -> i8 {
fn level_value(level: Option<Level>) -> u8 {
match level {
None => -1,
Some(Level::Error) => 0,
Some(Level::Warn) => 1,
Some(Level::Info) => 2,
Some(Level::Debug) => 3,
Some(Level::Trace) => 4,
None => 0,
Some(Level::Error) => 1,
Some(Level::Warn) => 2,
Some(Level::Info) => 3,
Some(Level::Debug) => 4,
Some(Level::Trace) => 5,
}
}

fn level_enum(verbosity: i8) -> Option<Level> {
fn level_enum(verbosity: u8) -> Option<Level> {
match verbosity {
i8::MIN..=-1 => None,
0 => Some(Level::Error),
1 => Some(Level::Warn),
2 => Some(Level::Info),
3 => Some(Level::Debug),
4..=i8::MAX => Some(Level::Trace),
0 => None,
1 => Some(Level::Error),
2 => Some(Level::Warn),
3 => Some(Level::Info),
4 => Some(Level::Debug),
5..=u8::MAX => Some(Level::Trace),
}
}

Expand Down Expand Up @@ -246,8 +248,10 @@ mod test {
(3, 0, Some(Level::Debug), LevelFilter::Debug),
(4, 0, Some(Level::Trace), LevelFilter::Trace),
(5, 0, Some(Level::Trace), LevelFilter::Trace),
(255, 0, Some(Level::Trace), LevelFilter::Trace),
(0, 1, None, LevelFilter::Off),
(0, 2, None, LevelFilter::Off),
(0, 255, None, LevelFilter::Off),
(255, 255, Some(Level::Error), LevelFilter::Error),
];

Expand All @@ -266,18 +270,6 @@ mod test {
}
}

/// These are characterization tests that show the current behavior
#[test]
fn verbosity_error_level_max_incorrect() {
let v: Verbosity<ErrorLevel> = Verbosity::new(255, 0);
assert_eq!(v.log_level(), None); // Should be Some(Level::Trace)
assert_eq!(v.log_level_filter(), LevelFilter::Off); // Should be LevelFilter::Trace

let v: Verbosity<ErrorLevel> = Verbosity::new(0, 255);
assert_eq!(v.log_level(), Some(Level::Warn)); // Should be None
assert_eq!(v.log_level_filter(), LevelFilter::Warn); // Should be LevelFilter::Off
}

#[test]
fn verbosity_warn_level() {
let tests = [
Expand All @@ -287,9 +279,11 @@ mod test {
(2, 0, Some(Level::Debug), LevelFilter::Debug),
(3, 0, Some(Level::Trace), LevelFilter::Trace),
(4, 0, Some(Level::Trace), LevelFilter::Trace),
(255, 0, Some(Level::Trace), LevelFilter::Trace),
(0, 1, Some(Level::Error), LevelFilter::Error),
(0, 2, None, LevelFilter::Off),
(0, 3, None, LevelFilter::Off),
(0, 255, None, LevelFilter::Off),
(255, 255, Some(Level::Warn), LevelFilter::Warn),
];

Expand All @@ -308,18 +302,6 @@ mod test {
}
}

/// These are characterization tests that show the current behavior
#[test]
fn verbosity_warn_level_max_incorrect() {
let v: Verbosity<WarnLevel> = Verbosity::new(255, 0);
assert_eq!(v.log_level(), Some(Level::Error)); // Should be Some(Level::Trace)
assert_eq!(v.log_level_filter(), LevelFilter::Error); // Should be LevelFilter::Trace

let v: Verbosity<WarnLevel> = Verbosity::new(0, 255);
assert_eq!(v.log_level(), Some(Level::Info)); // Should be None
assert_eq!(v.log_level_filter(), LevelFilter::Info); // Should be LevelFilter::Off
}

#[test]
fn verbosity_info_level() {
let tests = [
Expand All @@ -328,10 +310,12 @@ mod test {
(1, 0, Some(Level::Debug), LevelFilter::Debug),
(2, 0, Some(Level::Trace), LevelFilter::Trace),
(3, 0, Some(Level::Trace), LevelFilter::Trace),
(255, 0, Some(Level::Trace), LevelFilter::Trace),
(0, 1, Some(Level::Warn), LevelFilter::Warn),
(0, 2, Some(Level::Error), LevelFilter::Error),
(0, 3, None, LevelFilter::Off),
(0, 4, None, LevelFilter::Off),
(0, 255, None, LevelFilter::Off),
(255, 255, Some(Level::Info), LevelFilter::Info),
];

Expand All @@ -349,16 +333,4 @@ mod test {
);
}
}

/// These are characterization tests that show the current behavior
#[test]
fn verbosity_info_level_max_incorrect() {
let v: Verbosity<InfoLevel> = Verbosity::new(255, 0);
assert_eq!(v.log_level(), Some(Level::Warn)); // Should be Some(Level::Trace)
assert_eq!(v.log_level_filter(), LevelFilter::Warn); // Should be LevelFilter::Trace

let v: Verbosity<InfoLevel> = Verbosity::new(0, 255);
assert_eq!(v.log_level(), Some(Level::Debug)); // Should be None
assert_eq!(v.log_level_filter(), LevelFilter::Debug); // Should be LevelFilter::Off
}
}

0 comments on commit a7f7bb2

Please sign in to comment.