[patch] virtualbox/VBoxCreateUSBNode.sh test: integer expression expected -> fixed as of 7 Jun 2011 (not for version 4.0)

[ClemensFischer]

'uname -rms' -> Linux 2.6.38.7-spott-00008-g2232235 i686

virtualbox-4.0.8 on ARCH linux

This is in the logs when removing USB devices (example):

  daemon.info: Jun 2 19:35:30 udevd[28730]:
  '/usr/share/virtualbox/VBoxCreateUSBNode.sh 189 130 vboxusers'(err)
  '/usr/share/virtualbox/VBoxCreateUSBNode.sh: line 28: test: vboxusers:
  integer expression expected'

This patch will silence this unnecessary noise:

--- /usr/share/virtualbox/VBoxCreateUSBNode.sh.0        2011-05-18 13:55:16.000000000 +0200
+++ /usr/share/virtualbox/VBoxCreateUSBNode.sh  2011-06-02 19:51:57.566847667 +0200
@@ -25,9 +25,7 @@
 device=`expr "$2" '%' 128 + 1`
 class="$3"
 group="$4"
-if test "$class" -eq 9; then
-  exit 0
-fi
+case "-${class}" in -9) exit 0;; esac
 devdir="`printf "/dev/vboxusb/%.3d" $bus`"
 devpath="`printf "/dev/vboxusb/%.3d/%.3d" $bus $device`"
 if test "$do_remove" -eq 0; then

No need to mess with licensing for this one-liner, file it under MIT-licensed if you like.

--ClemensFischer

[Michael Thayer]

Thanks for pointing that out. I wonder though, is the "-" that you added still needed with "case"? "case" should do a textual/wildcard comparison, and "" should be a valid value for "$class". (Using "case" is certainly nicer than using "test", regardless of the integer issue.)

[ClemensFischer]

Replying to michael:

Thanks for pointing that out. I wonder though, is the "-" that you added still needed with "case"? "case" should do a textual/wildcard comparison, and "" should be a valid value for "$class". (Using "case" is certainly nicer than using "test", regardless of the integer issue.)

The hyphen makes it work on all bourne shells, even the ones where an empty "case" selector makes them hickup.

Using "case" can cause trouble though, compared to "test", depending on what is sent to the script: the shell and the "test" command know what to do with all sorts of numbers whereas "case" doesn't. If we get a decimal number with zeros prefixed, the decimal value is still the same, but the string pattern changes. In my scripts I use those posix patterns like case "-${number}" in -*([0])) ...;; esac, but this may require setting options making the construct less portable.

If you like, you can replace that "-" with an "X" as seen in many scripts. I just don't do this since (in "vim") there are commands like "*" and "#" that allow me to quickly find the pattern under the cursor at other places.

To make _me_ happy, you might want to give a name to that literal number "9", because nobody knows what it means, like:

bogus_dwoot_class=9
case "-${class}" in "-${bogus_dwoot_class}") exit 0;; esac

My scripts rarely contain literals other than in "declarations" where they are the values given to variables with descriptive names. This helps a lot when visiting a file after a long time and when memory may still serve names, but is lost with just numbers.

[Michael Thayer]

I committed the following change, which also fixes the real problem by moving the test for the device class into the right part of the code. I left out your dash prefix in the case statements as I don't think that they will be needed by any shell living on a system which also has udev. And regarding "test" and all sorts of numbers, try 'test "9.0" -eq "9"'! (It fails here at least.)

Note that I didn't backport this to 4.0 as it is a cosmetic issue.

$ svn di src/VBox/Installer/linux/VBoxCreateUSBNode.sh
Index: src/VBox/Installer/linux/VBoxCreateUSBNode.sh
===================================================================
--- src/VBox/Installer/linux/VBoxCreateUSBNode.sh	(revision 72097)
+++ src/VBox/Installer/linux/VBoxCreateUSBNode.sh	(working copy)
@@ -17,6 +17,9 @@
 # hope that it will be useful, but WITHOUT ANY WARRANTY of any kind.
 #
 
+# Constant, from the USB specifications
+usb_class_hub=9
+
 do_remove=0
 case "$1" in "--remove")
   do_remove=1; shift;;
@@ -25,22 +28,21 @@
 device=`expr "$2" '%' 128 + 1`
 class="$3"
 group="$4"
-if test "$class" -eq 9; then
-  exit 0
-fi
 devdir="`printf "/dev/vboxusb/%.3d" $bus`"
 devpath="`printf "/dev/vboxusb/%.3d/%.3d" $bus $device`"
-if test "$do_remove" -eq 0; then
-  if test -z "$group"; then
-    group="vboxusers"
-  fi
+case "$do_remove" in
+  0)
+  case "$class" in "$usb_class_hub") exit 0;; esac
+  case "$group" in "") group="vboxusers";; esac
   mkdir /dev/vboxusb -m 0750 2>/dev/null
   chown root:$group /dev/vboxusb 2>/dev/null
   mkdir "$devdir" -m 0750 2>/dev/null
   chown root:$group "$devdir" 2>/dev/null
   mknod "$devpath" c $1 $2 -m 0660 2>/dev/null
   chown root:$group "$devpath" 2>/dev/null
-else
+  ;;
+  1)
   rm -f "$devpath"
-fi
+  ;;
+esac