Re: [Qemu-devel] [RFC PATCH v3 03/11] Add a script to extract VSS SDK headers on POSIX system

[Eric Blake]

On 05/21/2013 09:33 AM, Tomoki Sekiyama wrote:
> VSS SDK(*) setup.exe is only runnable on Windows. This adds a script
> to extract VSS SDK headers on POSIX-systems using msitools.
> 
>   * http://www.microsoft.com/en-us/download/details.aspx?id=23490
> 
> From: Paolo Bonzini <address@hidden>
> Signed-off-by: Tomoki Sekiyama <address@hidden>
> ---
>  scripts/extract-vsssdk-headers |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100755 scripts/extract-vsssdk-headers
> 

> +#! /bin/bash

Since you are using bash...

> +
> +# extract-vsssdk-headers
> +# Author: Paolo Bonzini <address@hidden>
> +
> +set -e
> +if test $# = 0 || ! test -f "$1"; then

Why reject 0 arguments but not 2?  Shouldn't the first check be test $#
!= 1?

> +  echo 'Usage: extract-vsssdk-headers /path/to/setup.exe'
> +  exit 1
> +fi
> +
> +# Extract .MSI file in the .exe, looking for the OLE compound
> +# document signature.  Extra data at the end does not matter.
> +export LC_ALL=C
> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1'
> +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' `

...I'd prefer $() over ``.

> +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi
> +
> +# Now extract the files.
> +tmpdir=tmp$$
> +mkdir $tmpdir

...also, this name is rather predictable; adding a $RANDOM might help
improve its quality.

> +msiextract -C $tmpdir vsssdk.msi
> +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc
> +rm -rf $tmpdir vsssdk.msi

What, no trap, to guarantee clean up of the temp directory even if you
Ctrl-C the script in the middle of a potentially long-running msiextract?

-- 
Eric Blake   eblake redhat com    +1-919-301-3266
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature