From 5648d20add209107cd5bc1d167fc94355180daa9 Mon Sep 17 00:00:00 2001
From: mulhern <amulhern@redhat.com>
Date: Fri, 5 Jul 2024 14:37:47 -0400
Subject: [PATCH] Specify PathBuf type for blockdevs in parser

Note that this does virtually nothing. A PathBuf is basically just an
OsString with extra behaviors. And it can always be parsed from a
String, which is the clap default argument value type.

All this change accomplishes is that when processing the argument values,
the paths will not have to be converted to Strings and then Pathbufs
because that conversion happened when they were read, because clap was
instructed to do it.

This change just lays the foundation for good practice where it might be
noticeable, so that when clap makes it possible, other values, e.g., UUIDs,
can be checked for proper format by the parser, and if the format is wrong,
a parser error will be returned as part of the parser operation.

Signed-off-by: mulhern <amulhern@redhat.com>
---
 src/bin/stratis-min/stratis-min.rs | 37 +++++++++++++++++-------------
 1 file changed, 21 insertions(+), 16 deletions(-)

diff --git a/src/bin/stratis-min/stratis-min.rs b/src/bin/stratis-min/stratis-min.rs
index afaf60d0ba..5ec1b039ca 100644
--- a/src/bin/stratis-min/stratis-min.rs
+++ b/src/bin/stratis-min/stratis-min.rs
@@ -4,7 +4,7 @@
 
 use std::{error::Error, path::PathBuf};
 
-use clap::{Arg, ArgAction, ArgGroup, ArgMatches, Command};
+use clap::{Arg, ArgAction, ArgGroup, Command};
 use serde_json::{json, Map, Value};
 
 use stratisd::{
@@ -56,6 +56,7 @@ fn parse_args() -> Command {
                     .arg(
                         Arg::new("blockdevs")
                             .action(ArgAction::Append)
+                            .value_parser(clap::value_parser!(PathBuf))
                             .required(true),
                     )
                     .arg(Arg::new("key_desc").long("key-desc").num_args(1))
@@ -86,6 +87,7 @@ fn parse_args() -> Command {
                     .arg(
                         Arg::new("blockdevs")
                             .action(ArgAction::Append)
+                            .value_parser(clap::value_parser!(PathBuf))
                             .required(true),
                     ),
                 Command::new("rename")
@@ -96,6 +98,7 @@ fn parse_args() -> Command {
                     .arg(
                         Arg::new("blockdevs")
                             .action(ArgAction::Append)
+                            .value_parser(clap::value_parser!(PathBuf))
                             .required(true),
                     ),
                 Command::new("add-cache")
@@ -103,6 +106,7 @@ fn parse_args() -> Command {
                     .arg(
                         Arg::new("blockdevs")
                             .action(ArgAction::Append)
+                            .value_parser(clap::value_parser!(PathBuf))
                             .required(true),
                     ),
                 Command::new("destroy").arg(Arg::new("name").required(true)),
@@ -195,13 +199,6 @@ fn parse_args() -> Command {
         ])
 }
 
-fn get_paths_from_args(args: &ArgMatches) -> Vec<PathBuf> {
-    args.get_many::<String>("blockdevs")
-        .expect("required")
-        .map(PathBuf::from)
-        .collect::<Vec<_>>()
-}
-
 fn main() -> Result<(), String> {
     fn main_box() -> Result<(), Box<dyn Error>> {
         let cmd = parse_args();
@@ -261,7 +258,6 @@ fn main() -> Result<(), String> {
                 pool::pool_stop(id)?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("create") {
-                let paths = get_paths_from_args(args);
                 let key_description = match args.get_one::<String>("key_desc").map(|s| s.to_owned())
                 {
                     Some(string) => Some(KeyDescription::try_from(string)?),
@@ -294,7 +290,10 @@ fn main() -> Result<(), String> {
                 };
                 pool::pool_create(
                     args.get_one::<String>("name").expect("required").to_owned(),
-                    paths,
+                    args.get_many::<PathBuf>("blockdevs")
+                        .expect("required")
+                        .cloned()
+                        .collect::<Vec<_>>(),
                     EncryptionInfo::from_options((key_description, clevis_info)),
                 )?;
                 Ok(())
@@ -302,10 +301,12 @@ fn main() -> Result<(), String> {
                 pool::pool_destroy(args.get_one::<String>("name").expect("required").to_owned())?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("init-cache") {
-                let paths = get_paths_from_args(args);
                 pool::pool_init_cache(
                     args.get_one::<String>("name").expect("required").to_owned(),
-                    paths,
+                    args.get_many::<PathBuf>("blockdevs")
+                        .expect("required")
+                        .cloned()
+                        .collect::<Vec<_>>(),
                 )?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("rename") {
@@ -319,17 +320,21 @@ fn main() -> Result<(), String> {
                 )?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("add-data") {
-                let paths = get_paths_from_args(args);
                 pool::pool_add_data(
                     args.get_one::<String>("name").expect("required").to_owned(),
-                    paths,
+                    args.get_many::<PathBuf>("blockdevs")
+                        .expect("required")
+                        .cloned()
+                        .collect::<Vec<_>>(),
                 )?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("add-cache") {
-                let paths = get_paths_from_args(args);
                 pool::pool_add_cache(
                     args.get_one::<String>("name").expect("required").to_owned(),
-                    paths,
+                    args.get_many::<PathBuf>("blockdevs")
+                        .expect("required")
+                        .cloned()
+                        .collect::<Vec<_>>(),
                 )?;
                 Ok(())
             } else if let Some(args) = subcommand.subcommand_matches("is-encrypted") {