Ventoy icon indicating copy to clipboard operation
Ventoy copied to clipboard

Fix for "VentoyWorker.sh: 45: [: /dev/sdX: unexpected operator" and POSIX (1003.2) fixes

Open toazd opened this issue 5 years ago • 5 comments

This morning while upgrading my Ventoy disks from 1.0.27 to 1.0.28 I noticed I was getting a script error using any Ventoy function: ./tool/VentoyWorker.sh: 45: [: /dev/sdc: unexpected operator

Changing line 45 from: elif [ "$1" == "-h" ] || [ "$1" = "--help" ]; then to: elif [ "$1" = "-h" ] || [ "$1" = "--help" ]; then fixes the above mentioned issue. However, there are still a number of remaining issues, some potentially critical.

Using Shellcheck v0.7.1 I simply went down the list and addressed the issues one by one focusing on the ones that matter most first:

  1. Added quotes where needed when globbing/word-splitting may be undesired (will need review to make sure globbing/word-splitting isn't desired)
  2. Replace expr with $((..))
  3. Add -r to read
  4. echo "" and echo '' are the same as just plain echo (not an issue and I shouldn't have changed them, I just wanted to make you aware)
  5. Replace echo with printf when parameters are used (POSIX does not define -n for echo and this script and ventoy_lib.sh both misbehave when run in a POSIX environment - dash - without these fixes and the PR I posted regarding ventoy_lib.sh)

Note that there are now quotes in some locations where an IFS character would not be encountered (eg. /dev/sdX). I quoted them because it doesn't hurt to have them when you don't want globbing/word-splitting but it can cause a serious debugging headache without them.

There are still remaining issues that should be fixed IMHO. I did not want to choose how to approach the solution because this is not my project and there are multiple portable solutions for each issue. The main issues remaining are:

  1. SC2039: In POSIX sh, read -p is undefined.
  2. Line 270: SC2154: part2_start_sector is referenced but not assigned.

toazd avatar Nov 17 '20 13:11 toazd

Actually, VentoyWorker.sh was designed to work only under bash, not posix sh. It's called from Ventoy2Disk.sh as follows:

if [ -f /bin/bash ]; then
    bash ./tool/VentoyWorker.sh $*
else
    ./tool/ash ./tool/VentoyWorker.sh $*
fi

Is /bin/bash in your system a link to sh or dash ?

ventoy avatar Nov 17 '20 15:11 ventoy

Is /bin/bash in your system a link to sh or dash ?

Neither. bash links to /usr/bin/bash (/bin is a link to /usr/bin on Arch) and sh is linked to /usr/bin/dash (v0.5.11.2-1, latest on Arch).

toazd avatar Nov 18 '20 11:11 toazd

Is /bin/bash in your system a link to sh or dash ?

Neither. bash links to /usr/bin/bash (/bin is a link to /usr/bin on Arch) and sh is linked to /usr/bin/dash (v0.5.11.2-1, latest on Arch).

Which ISO do you use for your system? I will install the same distro and test it.

ventoy avatar Nov 18 '20 12:11 ventoy

Which ISO do you use for your system? I will install the same distro and test it.

Standard Arch ISO. Dash version 0.5.11.2-1

  • Install Dash (pactraparguments+= dash OR pacman -S dash after chroot)
  • Change /usr/bin/sh symlink to Dash (rm /usr/bin/sh && ln -s "$(which dash)" /usr/bin/sh)

toazd avatar Nov 18 '20 12:11 toazd

I cannot find CentOS-7-x86_64-Everything-2003.iso to test on: http://mirror.pit.teraswitch.com/centos/7/isos/x86_64/ but if it is available in the repos maybe sudo dnf install dash -y will work.

7.8.2003 path has only one file in it (other version have ISO) http://mirror.pit.teraswitch.com/centos/7.8.2003/readme that says: "This directory (and version of CentOS) is deprecated..."

Do you know a good source to download CentOS-7-x86_64-Everything-2003.iso ?

toazd avatar Nov 18 '20 13:11 toazd