Skip to content

Commit

Permalink
Merge pull request #3 from Apokleos/fix-clippy
Browse files Browse the repository at this point in the history
cdi-rs: fix clippy warnings and errors
  • Loading branch information
zvonkok authored May 17, 2024
2 parents 491fbcb + 8876f84 commit eddddbb
Show file tree
Hide file tree
Showing 8 changed files with 94 additions and 74 deletions.
13 changes: 9 additions & 4 deletions src/annotations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,18 @@ const MAX_NAME_LEN: usize = 63;
// injection request for the given devices. Upon any error a non-nil error
// is returned and annotations are left intact. By convention plugin should
// be in the format of "vendor.device-type".
#[allow(dead_code)]
pub(crate) fn update() {
println!("cdi::annotations::update");
}

// ParseAnnotations parses annotations for CDI device injection requests.
// The keys and devices from all such requests are collected into slices
// which are returned as the result. All devices are expected to be fully
// qualified CDI device names. If any device fails this check empty slices
// are returned along with a non-nil error. The annotations are expected
// to be formatted by, or in a compatible fashion to UpdateAnnotations().
#[allow(dead_code)]
pub(crate) fn parse_annotations(
annotations: HashMap<String, String>,
) -> Result<(Vec<String>, Vec<String>), anyhow::Error> {
Expand All @@ -32,21 +35,22 @@ pub(crate) fn parse_annotations(
}

for device in v.split(',') {
match parser::parse_qualified_name(device) {
Err(_) => return Err(anyhow!("invalid CDI device name {}", device)),
_ => (),
if let Err(_e) = parser::parse_qualified_name(device) {
return Err(anyhow!("invalid CDI device name {}", device));
}
devices.push(device.to_string());
}
keys.push(k);
}
Ok((keys, devices))
}

// AnnotationKey returns a unique annotation key for an device allocation
// by a K8s device plugin. pluginName should be in the format of
// "vendor.device-type". deviceID is the ID of the device the plugin is
// allocating. It is used to make sure that the generated key is unique
// even if multiple allocations by a single plugin needs to be annotated.
#[allow(dead_code)]
pub(crate) fn annotation_key(plugin_name: &str, device_id: &str) -> Result<String> {
if plugin_name.is_empty() {
return Err(anyhow!("invalid plugin name, empty"));
Expand All @@ -55,7 +59,7 @@ pub(crate) fn annotation_key(plugin_name: &str, device_id: &str) -> Result<Strin
return Err(anyhow!("invalid deviceID, empty"));
}

let name = plugin_name.to_owned() + "_" + &device_id.replace("/", "_");
let name = plugin_name.to_owned() + "_" + &device_id.replace('/', "_");

if name.len() > MAX_NAME_LEN {
return Err(anyhow!("invalid plugin+deviceID {:?}, too long", name));
Expand Down Expand Up @@ -101,6 +105,7 @@ pub(crate) fn annotation_key(plugin_name: &str, device_id: &str) -> Result<Strin
}

// AnnotationValue returns an annotation value for the given devices.
#[allow(dead_code)]
pub(crate) fn annotation_value(devices: Vec<String>) -> Result<String, anyhow::Error> {
devices.iter().try_for_each(|device| {
// Assuming parser::parse_qualified_name expects a &String and returns Result<(), Error>
Expand Down
10 changes: 6 additions & 4 deletions src/bin/validate.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
use anyhow::{anyhow, Result};
use anyhow::Result;
use clap::{App, Arg};
use std::io::{self, Read};

extern crate cdi;

use cdi::schema;
// use cdi::schema;

fn main() -> Result<()> {
let matches = App::new("validate")
Expand Down Expand Up @@ -44,14 +44,16 @@ fn main() -> Result<()> {

let document = matches.value_of("document").unwrap();

let doc_data = if document == "-" {
let _doc_data = if document == "-" {
println!("Reading from <stdin>...");
let mut buffer = Vec::new();
io::stdin().read_to_end(&mut buffer)?;
buffer
} else {
std::fs::read(&document)?
std::fs::read(document)?
};

//schema::validate(schema_file, &doc_data)

Ok(())
}
23 changes: 14 additions & 9 deletions src/cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ use std::collections::HashMap;

use crate::device::Device;
use crate::spec::Spec;
use crate::watch::Watch;
//use crate::watch::Watch;
use std::sync::{Arc, Mutex};

// CacheOption is an option to change some aspect of default CDI behavior.
Expand All @@ -24,6 +24,7 @@ impl CacheOption for WithAutoRefresh {
}
}

#[allow(dead_code)]
pub struct Cache {
spec_dirs: Vec<String>,
specs: HashMap<String, Vec<Spec>>,
Expand All @@ -32,7 +33,7 @@ pub struct Cache {
dir_errors: HashMap<String, Box<dyn std::error::Error + Send + Sync + 'static>>,

auto_refresh: bool,
watch: Watch,
//watch: Watch,
}

impl Cache {
Expand All @@ -44,28 +45,30 @@ impl Cache {
errors: HashMap::new(),
dir_errors: HashMap::new(),
auto_refresh: false,
watch: Watch::new(),
//watch: Watch::new(),
}))
}

pub fn configure(&mut self, options: Vec<Box<dyn CacheOption>>) {
for option in options {
option.apply(self);
}
}

pub fn list_vendors(&self) -> Vec<String> {
pub fn list_vendors(&mut self) -> Vec<String> {
let mut vendors: Vec<String> = Vec::new();

self.refresh_if_required(false);
let _ = self.refresh_if_required(false);

for vendor in self.specs.keys() {
vendors.push(*vendor);
vendors.push(vendor.clone());
}
vendors.sort();
vendors
}
pub fn get_vendor_specs(&self, vendor: &str) -> Vec<Spec> {
self.refresh_if_required(false);

pub fn get_vendor_specs(&mut self, vendor: &str) -> Vec<Spec> {
let _ = self.refresh_if_required(false);

match self.specs.get(vendor) {
Some(specs) => specs.clone(),
Expand All @@ -81,7 +84,9 @@ impl Cache {
// We need to refresh if
// - it's forced by an explicit call to Refresh() in manual mode
// - a missing Spec dir appears (added to watch) in auto-refresh mode
if force || (self.auto_refresh && self.watch.update(&mut self.dir_errors, vec![])) {
// TODO: Here it will be recoverd if watch is completed.
// if force || (self.auto_refresh && self.watch.update(&mut self.dir_errors, vec![])) {
if force || (self.auto_refresh) {
self.refresh()?;
return Ok(true);
}
Expand Down
1 change: 1 addition & 0 deletions src/device.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
// Device represents a CDI device of a Spec.
#[derive(Default)]
pub struct Device {
//*cdi.Device
//spec *Spec
Expand Down
4 changes: 2 additions & 2 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,13 +7,13 @@ pub mod parser;
pub mod registry;
pub mod schema;
pub mod spec;
pub mod watch;
//pub mod watch;
use crate::registry::RegistryCache;
use crate::registry::RegistrySpecDB;

pub fn cdi_list_vendors() {
let options: Vec<Box<dyn CacheOption>> = vec![Box::new(cache::WithAutoRefresh(true))];
let registry = registry::get_registry(options);
let registry = registry::get_registry(options).unwrap();
let vendors = registry.spec_db().list_vendors();

if vendors.is_empty() {
Expand Down
45 changes: 27 additions & 18 deletions src/parser.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,22 +12,26 @@ use anyhow::{anyhow, Result};
// A valid device name may contain the following runes:
//
// 'A'-'Z', 'a'-'z', '0'-'9', '-', '_', '.', ':'
#[allow(dead_code)]
pub(crate) fn qualified_name(vendor: &str, class: &str, name: &str) -> String {
format!("{}/{}={}", vendor, class, name)
}

// IsQualifiedName tests if a device name is qualified.
#[allow(dead_code)]
pub(crate) fn is_qualified_name(name: &str) -> bool {
match parse_qualified_name(name) {
Ok(_) => {
print!("{} is a qualified name\n", name);
println!("{} is a qualified name", name);
true
}
Err(e) => {
println!("{} is not a qualified name, {}\n", name, e);
println!("{} is not a qualified name, {}", name, e);
false
}
}
}

// ParseQualifiedName splits a qualified name into device vendor, class,
// and name. If the device fails to parse as a qualified name, or if any
// of the split components fail to pass syntax validation, vendor and
Expand All @@ -46,26 +50,24 @@ pub(crate) fn parse_qualified_name(
if name.is_empty() {
return Err(anyhow!("unqualified device {}, missing name", device));
}
match validate_vendor_name(vendor) {
Err(e) => return Err(anyhow!("invalid device {}: {}", device, e)),
_ => (),
if let Err(e) = validate_vendor_name(vendor) {
return Err(anyhow!("invalid vendor {}: {}", device, e));
}
match validate_class_name(class) {
Err(e) => return Err(anyhow!("invalid device {}: {}", device, e)),
_ => (),
if let Err(e) = validate_class_name(class) {
return Err(anyhow!("invalid class {}: {}", device, e));
}
match validate_device_name(name) {
Err(e) => return Err(anyhow!("invalid device {}: {}", device, e)),
_ => (),
if let Err(e) = validate_device_name(name) {
return Err(anyhow!("invalid device {}: {}", device, e));
}
Ok((vendor.to_string(), class.to_string(), name.to_string()))
}

// ParseDevice tries to split a device name into vendor, class, and name.
// If this fails, for instance in the case of unqualified device names,
// ParseDevice returns an empty vendor and class together with name set
// to the verbatim input.
pub(crate) fn parse_device(device: &str) -> (&str, &str, &str) {
if device.is_empty() || device.chars().next().unwrap() == '/' {
if device.is_empty() || device.starts_with('/') {
return ("", "", device);
}

Expand All @@ -81,6 +83,7 @@ pub(crate) fn parse_device(device: &str) -> (&str, &str, &str) {
}
(vendor, class, name)
}

// ParseQualifier splits a device qualifier into vendor and class.
// The syntax for a device qualifier is
//
Expand All @@ -95,28 +98,33 @@ pub(crate) fn parse_qualifier(kind: &str) -> (&str, &str) {
}
(parts[0], parts[1])
}

// ValidateVendorName checks the validity of a vendor name.
// A vendor name may contain the following ASCII characters:
// - upper- and lowercase letters ('A'-'Z', 'a'-'z')
// - digits ('0'-'9')
// - underscore, dash, and dot ('_', '-', and '.')
pub(crate) fn validate_vendor_name(vendor: &str) -> Result<()> {
match validate_vendor_or_class_name(vendor) {
Err(e) => Err(anyhow!("invalid vendor. {}", e)),
_ => Ok(()),
if let Err(e) = validate_vendor_or_class_name(vendor) {
return Err(anyhow!("invalid vendor. {}", e))
}

Ok(())
}

// ValidateClassName checks the validity of class name.
// A class name may contain the following ASCII characters:
// - upper- and lowercase letters ('A'-'Z', 'a'-'z')
// - digits ('0'-'9')
// - underscore, dash, and dot ('_', '-', and '.')
pub(crate) fn validate_class_name(class: &str) -> Result<()> {
match validate_vendor_or_class_name(class) {
Err(e) => Err(anyhow!("invalid class. {}", e)),
_ => Ok(()),
if let Err(e) = validate_vendor_or_class_name(class) {
return Err(anyhow!("invalid class. {}", e));
}

Ok(())
}

// validateVendorOrClassName checks the validity of vendor or class name.
// A name may contain the following ASCII characters:
// - upper- and lowercase letters ('A'-'Z', 'a'-'z')
Expand All @@ -137,6 +145,7 @@ pub(crate) fn validate_vendor_or_class_name(name: &str) -> Result<()> {
}
Ok(())
}

// ValidateDeviceName checks the validity of a device name.
// A device name may contain the following ASCII characters:
// - upper- and lowercase letters ('A'-'Z', 'a'-'z')
Expand Down
Loading

0 comments on commit eddddbb

Please sign in to comment.