Use a static map to lookup which characters are allowed

Also improves the efficiency of descriptor checksum code.

Author: sanket1729

src/descriptor/checksum.rs

@@ -8,7 +8,7 @@
 use core::fmt;
 use core::iter::FromIterator;
 
-pub use crate::expression::INPUT_CHARSET;
+pub use crate::expression::VALID_CHARS;
 use crate::prelude::*;
 use crate::Error;
 
@@ -101,9 +101,14 @@ impl Engine {
     /// state! It is safe to continue feeding it data but the result will not be meaningful.
     pub fn input(&mut self, s: &str) -> Result<(), Error> {
         for ch in s.chars() {
-            let pos = INPUT_CHARSET.find(ch).ok_or_else(|| {
-                Error::BadDescriptor(format!("Invalid character in checksum: '{}'", ch))
-            })? as u64;
+            let pos = VALID_CHARS
+                .get(ch as usize)
+                .ok_or_else(|| {
+                    Error::BadDescriptor(format!("Invalid character in checksum: '{}'", ch))
+                })?
+                .ok_or_else(|| {
+                    Error::BadDescriptor(format!("Invalid character in checksum: '{}'", ch))
+                })? as u64;
             self.c = poly_mod(self.c, pos & 31);
             self.cls = self.cls * 3 + (pos >> 5);
             self.clscount += 1;

src/expression.rs

@@ -11,6 +11,26 @@ use crate::{errstr, Error, MAX_RECURSION_DEPTH};
 /// Allowed characters are descriptor strings.
 pub const INPUT_CHARSET: &str =  "0123456789()[],'/*abcdefgh@:$%{}IJKLMNOPQRSTUVWXYZ&+-.;<=>?!^_|~ijklmnopqrstuvwxyzABCDEFGH`#\"\\ ";
 
+/// Map of valid characters in descriptor strings.
+#[rustfmt::skip]
+pub const VALID_CHARS: [Option<u8>; 128] = [
+    None, None, None, None, None, None, None, None, None, None, None, None, None,
+    None, None, None, None, None, None, None, None, None, None, None, None, None,
+    None, None, None, None, None, None, Some(94), Some(59), Some(92), Some(91),
+    Some(28), Some(29), Some(50), Some(15), Some(10), Some(11), Some(17), Some(51),
+    Some(14), Some(52), Some(53), Some(16), Some(0), Some(1), Some(2), Some(3),
+    Some(4), Some(5), Some(6), Some(7), Some(8), Some(9), Some(27), Some(54),
+    Some(55), Some(56), Some(57), Some(58), Some(26), Some(82), Some(83),
+    Some(84), Some(85), Some(86), Some(87), Some(88), Some(89), Some(32), Some(33),
+    Some(34), Some(35), Some(36), Some(37), Some(38), Some(39), Some(40), Some(41),
+    Some(42), Some(43), Some(44), Some(45), Some(46), Some(47), Some(48), Some(49),
+    Some(12), Some(93), Some(13), Some(60), Some(61), Some(90), Some(18), Some(19),
+    Some(20), Some(21), Some(22), Some(23), Some(24), Some(25), Some(64), Some(65),
+    Some(66), Some(67), Some(68), Some(69), Some(70), Some(71), Some(72), Some(73),
+    Some(74), Some(75), Some(76), Some(77), Some(78), Some(79), Some(80), Some(81),
+    Some(30), Some(62), Some(31), Some(63), None,
+];
+
 #[derive(Debug)]
 /// A token of the form `x(...)` or `x`
 pub struct Tree<'a> {
@@ -187,10 +207,12 @@ pub fn check_valid_chars(s: &str) -> Result<(), Error> {
         if !ch.is_ascii() {
             return Err(Error::Unprintable(ch));
         }
-        // TODO: Avoid linear search overhead by using OnceCell to cache this in a BTreeMap.
-        INPUT_CHARSET
-            .find(char::from(ch))
-            .ok_or_else(|| Error::Unprintable(ch))?;
+        // Index bounds: We know that ch is ASCII, so it is <= 127.
+        if VALID_CHARS[ch as usize].is_none() {
+            return Err(Error::Unexpected(
+                "Only characters in INPUT_CHARSET are allowed".to_string(),
+            ));
+        }
     }
     Ok(())
 }
@@ -265,4 +287,13 @@ mod tests {
         assert!(parse_num("+6").is_err());
         assert!(parse_num("-6").is_err());
     }
+
+    #[test]
+    fn test_valid_char_map() {
+        let mut valid_chars = [None; 128];
+        for (i, ch) in super::INPUT_CHARSET.chars().enumerate() {
+            valid_chars[ch as usize] = Some(i as u8);
+        }
+        assert_eq!(valid_chars, super::VALID_CHARS);
+    }
 }