Code review comment for lp:~gl-az/percona-xtrabackup/bug1243009-2.1

Vlad Lesin (vlad-lesin) wrote :

The same questions as here: https://code.launchpad.net/~gl-az/percona-xtrabackup/bug1243009-2.0/+merge/195434/comments/465887.

I am not sure in correctness if this code in test/t/version_check.sh:

+for ((i=0; i < ${#IB_ARGS[@]}; i++))
+do
+ if [ "${IB_ARGS[$i]}" = "--no-version-check" ]
+ then
+ IB_ARGS=("${IB_ARGS[@]:0:$i}" "${IB_ARGS[@]:(($i + 1))}")
+ fi
+done

Imagine you have three "--no-version-check" options in a line starting with index say 2. There will be the following iterations:

1)
before removing
index 2 3 4
i = 2, IB_ARGS = (... "--no-version-check" "--no-version-check" "--no-version-check" ...), ${#IB_ARGS[@]} = n
after removing
index 2 3
i = 2, IB_ARGS = (... "--no-version-check" "--no-version-check" ...), ${#IB_ARGS[@]} = n - 1

2)
before removing
index 2 3
i = 3, IB_ARGS = (... "--no-version-check" "--no-version-check" ...), ${#IB_ARGS[@]} = n - 1
after removing
index 2 3
i = 3, IB_ARGS = (... "--no-version-check" ... ...), ${#IB_ARGS[@]} = n - 2

So as we can see this algorithm removes only a half of "--no-version-check" elements because $i is increased on every loop but the tail of the array is shifted by one element left and the index of the next element after removed element is decreased.

review: Needs Fixing (g2)

« Back to merge proposal