On 2.11.2017 23:12, Dave Chinner wrote: > On Thu, Nov 02, 2017 at 10:13:41AM +0200, Nikolay Borisov wrote: >> From: Nikolay Borsiov <nborisov@xxxxxxxx> >> >> Signed-off-by: Nikolay Borisov <nborisov@xxxxxxxx> >> --- >> v3: >> * Changed the way we detect ranged args. Now use a regexp which checks >> explicitly for the ranged args >> common/rc | 7 +++++++ >> 1 file changed, 7 insertions(+) >> >> diff --git a/common/rc b/common/rc >> index e2a8229..f7a5fe9 100644 >> --- a/common/rc >> +++ b/common/rc >> @@ -2053,8 +2053,15 @@ _require_xfs_io_command() >> -c "$command 4k 8k" $testfile 2>&1` >> ;; >> "fiemap") >> + if echo "$param" | egrep -q "[[:digit:]]+[bskmgtpe]? [[:digit:]]+[bskmgtpe]?$" > > What is that for? > > If you're going to dump crazy complex regexes to match something, > you need comments to explain it in both the commit message and the > code. > > And, really, if we need to add complex or tricky code to check a > command exists, we've done somethign wrong. Don't make the "fiemap > range" command by triggered/detected by an optional parameter, add a > specific option instead e.g. "-r" for "range query": > > $ xfs_io -c "fiemap -r 0 10k" /mnt/foo Do you mean having the range params as arguments to the -r option (which would be a bitch to parse since getopt doesn't support multiple args to an options). Or having the -r serve as a boolean indicating we'd need to parse the final 2 args (as it's done currently)? > > Then you can test it like the falloc command tests it's different > parameters.... > > -Dave. > -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
