pr feedback

src/error.rs

@@ -2,10 +2,12 @@ use thiserror::Error;
 
 #[derive(Error, Debug)]
 pub enum EtwError {
-    #[error("Provider group GUID must not be zeroes")]
+    #[error("Provider group GUID must not be zeros")]
     EmptyProviderGroupGuid,
     #[error("Provider group names must be lower case ASCII or numeric digits: {0:?}")]
     InvalidProviderGroupCharacters(String),
     #[error("Linux provider names must be ASCII alphanumeric: {0:?}")]
-    InvalidProviderNameCharacters(String)
+    InvalidProviderNameCharacters(String),
+    #[error("Linux provider name and provider group must less than 234 characters combined. Current length: {0:?}")]
+    TooManyCharacters(usize)
 }

src/layer.rs

@@ -128,13 +128,24 @@ where
 
     fn validate_config(&self) -> Result<(), EtwError> {
         #[cfg(target_os = "linux")]
-        if self
-            .provider_name
-            .contains(|f: char| !f.is_ascii_alphanumeric())
         {
-            // The perf command is very particular about the provider names it accepts.
-            // The Linux kernel itself cares less, and other event consumers should also presumably not need this check.
-            return Err(EtwError::InvalidProviderNameCharacters(self.provider_name.clone()));
+            if self
+                .provider_name
+                .contains(|f: char| !f.is_ascii_alphanumeric() && f != '_')
+            {
+                // The perf command is very particular about the provider names it accepts.
+                // The Linux kernel itself cares less, and other event consumers should also presumably not need this check.
+                return Err(EtwError::InvalidProviderNameCharacters(self.provider_name.clone()));
+            }
+
+            let group_name_len = match &self.provider_group {
+                None => 0,
+                Some(ref name) => Mode::get_provider_group(&name).as_ref().len()
+            };
+
+            if self.provider_name.len() + group_name_len >= 234 {
+                return Err(EtwError::TooManyCharacters(self.provider_name.len() + group_name_len));
+            }
         }
 
         match &self.provider_group {
@@ -150,7 +161,7 @@ where
         match self.provider_group {
             None => {}
             Some(ref name) => {
-                targets = targets.with_target(Mode::get_provider_group(name), LevelFilter::TRACE);
+                targets = targets.with_target(Mode::get_provider_group(name).as_ref(), LevelFilter::TRACE);
             }
         }
 

src/native/common_schema/user_events_cs.rs

@@ -68,7 +68,7 @@ impl crate::native::ProviderTypes for CommonSchemaProvider {
         }
     }
 
-    fn get_provider_group(value: &Self::ProviderGroupType) -> impl Into<String> {
+    fn get_provider_group(value: &Self::ProviderGroupType) -> impl AsRef<str> {
         value.clone()
     }
 }

src/native/mod.rs

@@ -100,7 +100,7 @@ pub trait ProviderTypes {
     // that it trivially matches a constraint unless we lower the constraint
     // checking into the impl, done here through constraint on the return type.
     #[cfg(target_os = "linux")]
-    fn get_provider_group(value: &Self::ProviderGroupType) -> impl Into<String>;
+    fn get_provider_group(value: &Self::ProviderGroupType) -> impl AsRef<str>;
 }
 
 #[doc(hidden)]

src/native/user_events.rs

@@ -75,7 +75,7 @@ impl crate::native::ProviderTypes for Provider {
         }
     }
 
-    fn get_provider_group(value: &Self::ProviderGroupType) -> impl Into<String> {
+    fn get_provider_group(value: &Self::ProviderGroupType) -> impl AsRef<str> {
         value.clone()
     }
 }