On Wed, Apr 15, 2015 at 08:28:21PM +0800, Zhao Lei wrote: > Hi, Dave Chinner > > > -----Original Message----- > > From: Dave Chinner [mailto:[email protected]] > > Sent: Wednesday, April 15, 2015 5:45 AM > > To: Zhaolei > > Cc: [email protected] > > Subject: Re: [PATCH v2] Fix caller's argument for _require_command() > > > > On Mon, Apr 13, 2015 at 12:32:43PM +0800, Zhaolei wrote: > > > From: Zhao Lei <[email protected]> > > > > > > _require_command() only accept 2 arguments, first one is pure command, > > > and second one is name for error message. > > > > > > But some caller misused this function, for example, > > > DEFRAG_PROG="$BTRFS_UTIL_PROG filesystem defragment" > > > _require_command $DEFRAG_PROG defragment In above code, > > > _require_command get 4 arguments, the caller's second argument is > > > never used, and the second field of first argument is treat as "error > > > message" in _require_command. > > > > > > We fixed above case by adding quotes to _require_command()'s > > > arguments, it fixed most test case, but introduced a new bug, in above > > > case, "btrfs filesystem defragment" is not a command, and always > > > failed in _require_command(), it caused some testcase not work, as > > > btrfs/005. > > > > > > This patch fixed above caller. > > > > > > Changelog v1->v2: > > > 1: Add detail description, suggested by: > > > Lukáš Czerner <[email protected]> > > > 2: Add missed Reported-by. > > > 3: Make $XFSDUMP_PROG always be a pure command, to avoid special > > > handling for it. > > > > Having to change xfsdump checks means this is still the wrong fix. > > > > _require_command should simply handle multi-part command strings. > > > > Does the following patch fix your problems? > > > This patch can deal with current code, only can't deal with program with > blank in filename. > But this is rarely happened, so we need not care about it.
It will work just fine with a minor tweak: $ [ -x /bin/true ] && echo foo foo $ [ -x "" ] && echo foo $ i.e. the -x check fails as expected when passed an empty command. > > + if [ ! -x $command ]; then i.e this needs changing to [ ! -x "$command" ].... Cheers, Dave. -- Dave Chinner [email protected] -- To unsubscribe from this list: send the line "unsubscribe fstests" in the body of a message to [email protected] More majordomo info at http://vger.kernel.org/majordomo-info.html
