| Message ID | c0ebb36f51f97acb3612ec5376a68441b5e62ac6.1695383055.git.fdmanana@suse.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | btrfs: use full subcommand name at _btrfs_get_subvolid() | expand |
On Fri, 22 Sep 2023 12:45:01 +0100, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > Avoid using the shortcut "sub" for the "subvolume" command, as this is the > standard practice because such shortcuts are not guaranteed to exist in > every btrfs-progs release (they may come and go). Also make the variables > local. Hmm, having them come and go would likely break quite a few user scripts... > > Signed-off-by: Filipe Manana <fdmanana@suse.com> > --- > common/btrfs | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/btrfs b/common/btrfs > index c9903a41..62cee209 100644 > --- a/common/btrfs > +++ b/common/btrfs > @@ -6,10 +6,10 @@ > > _btrfs_get_subvolid() > { > - mnt=$1 > - name=$2 > + local mnt=$1 > + local name=$2 > > - $BTRFS_UTIL_PROG sub list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' > + $BTRFS_UTIL_PROG subvolume list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' My grep+awk OCD would say that this should be: $BTRFS_UTIL_PROG subvolume list $mnt | $AWK_PROG -v name="$name" '$9 == name { print $2 }' Looks fine as is though. Reviewed-by: David Disseldorp <ddiss@suse.de>
On 2023/9/22 21:15, fdmanana@kernel.org wrote: > From: Filipe Manana <fdmanana@suse.com> > > Avoid using the shortcut "sub" for the "subvolume" command, as this is the > standard practice because such shortcuts are not guaranteed to exist in > every btrfs-progs release (they may come and go). Also make the variables > local. > > Signed-off-by: Filipe Manana <fdmanana@suse.com> Reviewed-by: Qu Wenruo <wqu@suse.com> Thanks, Qu > --- > common/btrfs | 6 +++--- > 1 file changed, 3 insertions(+), 3 deletions(-) > > diff --git a/common/btrfs b/common/btrfs > index c9903a41..62cee209 100644 > --- a/common/btrfs > +++ b/common/btrfs > @@ -6,10 +6,10 @@ > > _btrfs_get_subvolid() > { > - mnt=$1 > - name=$2 > + local mnt=$1 > + local name=$2 > > - $BTRFS_UTIL_PROG sub list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' > + $BTRFS_UTIL_PROG subvolume list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' > } > > # _require_btrfs_command <command> [<subcommand>|<option>]
On Fri, Sep 22, 2023 at 05:18:35PM +0200, David Disseldorp wrote: > On Fri, 22 Sep 2023 12:45:01 +0100, fdmanana@kernel.org wrote: > > > From: Filipe Manana <fdmanana@suse.com> > > > > Avoid using the shortcut "sub" for the "subvolume" command, as this is the > > standard practice because such shortcuts are not guaranteed to exist in > > every btrfs-progs release (they may come and go). Also make the variables > > local. > > Hmm, having them come and go would likely break quite a few user > scripts... The help text of 'btrfs' says at the end: "For an overview of a given command use 'btrfs command --help' or 'btrfs [command...] --help --full' to print all available options. Any command name can be shortened as far as it stays unambiguous, however it is recommended to use full command names in scripts. ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ All command groups have their manual page named 'btrfs-<group>'." The commands will not disappear but new commands may be added and the prefix would become ambiguous. The short versions are accepted for manual command typing convenience, scripts should use the full length.
diff --git a/common/btrfs b/common/btrfs index c9903a41..62cee209 100644 --- a/common/btrfs +++ b/common/btrfs @@ -6,10 +6,10 @@ _btrfs_get_subvolid() { - mnt=$1 - name=$2 + local mnt=$1 + local name=$2 - $BTRFS_UTIL_PROG sub list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' + $BTRFS_UTIL_PROG subvolume list $mnt | grep -E "\s$name$" | $AWK_PROG '{ print $2 }' } # _require_btrfs_command <command> [<subcommand>|<option>]