[PATCH] btrfs: use full subcommand name at _btrfs_get_subvolid()

[PATCH] btrfs: use full subcommand name at _btrfs_get_subvolid()

[fdmanana]

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>
---
 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>]
-- 
2.40.1

Re: [PATCH] btrfs: use full subcommand name at _btrfs_get_subvolid()

[David Disseldorp]

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>

Re: [PATCH] btrfs: use full subcommand name at _btrfs_get_subvolid()

[Qu Wenruo]

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>]

Re: [PATCH] btrfs: use full subcommand name at _btrfs_get_subvolid()

[David Sterba]

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.