[v1] tools: fix iterating over argv when recovering xattr

[PATCH] tools: fix iterating over argv when recovering xattr

Daniel P. Berrangé posted 1 patch 2 years, 5 months ago

Diff against v2
Download mbox

Test syntax-check failed

Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20211126133817.1626274-1-berrange@redhat.com

There is a newer version of this series

tools/libvirt_recover_xattrs.sh | 7 +++----
1 file changed, 3 insertions(+), 4 deletions(-)

Expand all Fold all

[PATCH] tools: fix iterating over argv when recovering xattr

Posted by Daniel P. Berrangé 2 years, 5 months ago

The libvirt_recover_xattrs.sh tool hangs when run. It appears that
calling 'shift' is not modifying the '$#' value, so the loop never
terminates. Rewrite to just loop over $@ instead which involves less
cleverness.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 tools/libvirt_recover_xattrs.sh | 7 +++----
 1 file changed, 3 insertions(+), 4 deletions(-)

diff --git a/tools/libvirt_recover_xattrs.sh b/tools/libvirt_recover_xattrs.sh
index be6ee84b5f..ae9a18bad8 100755
--- a/tools/libvirt_recover_xattrs.sh
+++ b/tools/libvirt_recover_xattrs.sh
@@ -104,11 +104,10 @@ fix_xattrs() {
 }
 
 
-shift $((OPTIND - 1))
 if [ $# -gt 0 ]; then
-    while [ $# -gt 0 ]; do
-        fix_xattrs "$1"
-        shift $((OPTIND - 1))
+    for arg in "$@"
+    do
+        fix_xattrs "$arg"
     done
 else
     if [ ${UNSAFE} -eq 1 ]; then
-- 
2.33.1

Re: [PATCH] tools: fix iterating over argv when recovering xattr

Posted by Martin Kletzander 2 years, 5 months ago

On Fri, Nov 26, 2021 at 01:38:17PM +0000, Daniel P. Berrangé wrote:
>The libvirt_recover_xattrs.sh tool hangs when run. It appears that
>calling 'shift' is not modifying the '$#' value, so the loop never
>terminates. Rewrite to just loop over $@ instead which involves less
>cleverness.
>

shift actually does modify the value of the '#' variable, but if OPTIND
is 1, then the shift is called as `shift 0` which, of course, does not
shift any values (shifts them by 0).

>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> tools/libvirt_recover_xattrs.sh | 7 +++----
> 1 file changed, 3 insertions(+), 4 deletions(-)
>
>diff --git a/tools/libvirt_recover_xattrs.sh b/tools/libvirt_recover_xattrs.sh
>index be6ee84b5f..ae9a18bad8 100755
>--- a/tools/libvirt_recover_xattrs.sh
>+++ b/tools/libvirt_recover_xattrs.sh
>@@ -104,11 +104,10 @@ fix_xattrs() {
> }
>
>
>-shift $((OPTIND - 1))

I think this needs to stay here in case there are some parameters that
were not shifted as it looks like that's the reason for using OPTIND
here.

> if [ $# -gt 0 ]; then
>-    while [ $# -gt 0 ]; do
>-        fix_xattrs "$1"
>-        shift $((OPTIND - 1))

I think the idea was that this shift should've just happen without
parameters to default to `shift 1`.  But `for` works too.

>+    for arg in "$@"
>+    do
>+        fix_xattrs "$arg"
>     done
> else
>     if [ ${UNSAFE} -eq 1 ]; then
>-- 
>2.33.1
>