Patchwork [RFC,v3,03/11] Add a script to extract VSS SDK headers on POSIX system

login
register
mail settings
Submitter Tomoki Sekiyama
Date May 21, 2013, 3:33 p.m.
Message ID <20130521153345.4880.68009.stgit@hds.com>
Download mbox | patch
Permalink /patch/245326/
State New
Headers show

Comments

Tomoki Sekiyama - May 21, 2013, 3:33 p.m.
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 <pbonzini@redhat.com>
Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
---
 scripts/extract-vsssdk-headers |   25 +++++++++++++++++++++++++
 1 file changed, 25 insertions(+)
 create mode 100755 scripts/extract-vsssdk-headers
Eric Blake - May 21, 2013, 4:48 p.m.
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 <pbonzini@redhat.com>
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
> ---
>  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 <pbonzini@redhat.com>
> +
> +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?
Tomoki Sekiyama - May 21, 2013, 9:02 p.m.
On 5/21/13 12:48 , "Eric Blake" <eblake@redhat.com> wrote:
>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 <pbonzini@redhat.com>
>> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
>> ---
>>  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 <pbonzini@redhat.com>
>> +
>> +set -e
>> +if test $# = 0 || ! test -f "$1"; then
>
>Why reject 0 arguments but not 2?  Shouldn't the first check be test $#
>!= 1?

I agree, will fix this.

>> +  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 ``.

OK.

>> +(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.

I will add $RANDOM here.

>> +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?

OK, I will add some trap here.

Maybe it also should have an additional check and a message to install
Msitools for the case msiextract isn't exectable.

Thanks,
Laszlo Ersek - May 24, 2013, 1:14 p.m.
On 05/21/13 23:02, Tomoki Sekiyama wrote:

> Maybe it also should have an additional check and a message to install
> Msitools for the case msiextract isn't exectable.

Right. One POSIX-y way would be

  command -v msiextract >/dev/null

and checking the exit status.

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/command.html

Alternatively, just go ahead calling it, and if the exit status is 126
or 127, assume it is not (correctly) installed and emit an extra hint
afterwards:

http://pubs.opengroup.org/onlinepubs/9699919799/utilities/V3_chap02.html#tag_18_09_01_01

Laszlo
Laszlo Ersek - May 24, 2013, 1:38 p.m.
On 05/21/13 17:33, 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 <pbonzini@redhat.com>
> Signed-off-by: Tomoki Sekiyama <tomoki.sekiyama@hds.com>
> ---
>  scripts/extract-vsssdk-headers |   25 +++++++++++++++++++++++++
>  1 file changed, 25 insertions(+)
>  create mode 100755 scripts/extract-vsssdk-headers
> 
> diff --git a/scripts/extract-vsssdk-headers b/scripts/extract-vsssdk-headers
> new file mode 100755
> index 0000000..5877137
> --- /dev/null
> +++ b/scripts/extract-vsssdk-headers
> @@ -0,0 +1,25 @@
> +#! /bin/bash
> +
> +# extract-vsssdk-headers
> +# Author: Paolo Bonzini <pbonzini@redhat.com>
> +
> +set -e
> +if test $# = 0 || ! test -f "$1"; then
> +  echo 'Usage: extract-vsssdk-headers /path/to/setup.exe'
> +  exit 1
> +fi

Error message to >&2 ?

(Would not be worth a repost but you're already doing one...)

> +
> +# 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'

Can't help mentioning the following portable (alas, octal) equivalent :)

MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341')

<http://pubs.opengroup.org/onlinepubs/9699919799/utilities/printf.html#tag_20_94>

> +offset=`grep -abom1 "$MAGIC" "$1" | sed -n 's/:/\n/; P' `

Of these grep options, none are portable, hence I'm supposing dependency
on GNU grep. In that case, see (*).

> +(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi

In place of dd for seeking + cat, suggest

  tail -c +$((offset+1)) -- "$1"

<http://pubs.opengroup.org/onlinepubs/9699919799/utilities/tail.html>

> +
> +# Now extract the files.
> +tmpdir=tmp$$
> +mkdir $tmpdir
> +msiextract -C $tmpdir vsssdk.msi
> +mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc
> +rm -rf $tmpdir vsssdk.msi
> +exit 0

(*) Since we rely on GNU utilities anyway, I propose the mktemp utility
("Written by Jim Meyering and Eric Blake", heh) from GNU coreutils:

tmpdir=$(mktemp -d)


Feel free to ignore any of the above, of course :)

Thanks,
Laszlo
Eric Blake - May 24, 2013, 3:59 p.m.
On 05/24/2013 07:38 AM, Laszlo Ersek wrote:

>> +++ b/scripts/extract-vsssdk-headers
>> @@ -0,0 +1,25 @@
>> +#! /bin/bash
>> +

>> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1'
> 
> Can't help mentioning the following portable (alas, octal) equivalent :)
> 
> MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341')

Yeah, but as long as the she-bang is (correctly) requiring bash, we
might as well use the full power of bash.  Also, Issue 8 of POSIX will
be mandating $'' handling (but it could be several years before Issue 8
becomes a standard, compared to Issue 7 [aka POSIX 2008] just barely
released its first technical corrigendum [aka POSIX 2013]).
Laszlo Ersek - May 24, 2013, 6:27 p.m.
On 05/24/13 17:59, Eric Blake wrote:
> On 05/24/2013 07:38 AM, Laszlo Ersek wrote:
> 
>>> +++ b/scripts/extract-vsssdk-headers
>>> @@ -0,0 +1,25 @@
>>> +#! /bin/bash
>>> +
> 
>>> +MAGIC=$'\xd0\xcf\x11\xe0\xa1\xb1\x1a\xe1'
>>
>> Can't help mentioning the following portable (alas, octal) equivalent :)
>>
>> MAGIC=$(printf '%b' '\0320\0317\0021\0340\0241\0261\0032\0341')
> 
> Yeah, but as long as the she-bang is (correctly) requiring bash,

Yes, that's why I edited my original "please consider using" to "can't
help mentioning" :) I recalled that you had mentioned dash in one of
your reviews (*), I checked your reply to this v3 03/11 and saw that it
wasn't it -- here you'd written "Since you are using bash" so I edited
the above paragraph. Clearly insufficiently :)

(*) ... Apparently you mentioned dash in a MALLOC_PERTURB_ thread.

L.

Patch

diff --git a/scripts/extract-vsssdk-headers b/scripts/extract-vsssdk-headers
new file mode 100755
index 0000000..5877137
--- /dev/null
+++ b/scripts/extract-vsssdk-headers
@@ -0,0 +1,25 @@ 
+#! /bin/bash
+
+# extract-vsssdk-headers
+# Author: Paolo Bonzini <pbonzini@redhat.com>
+
+set -e
+if test $# = 0 || ! test -f "$1"; then
+  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' `
+(dd of=/dev/null skip=$offset bs=1 count=0; cat) < "$1" > vsssdk.msi
+
+# Now extract the files.
+tmpdir=tmp$$
+mkdir $tmpdir
+msiextract -C $tmpdir vsssdk.msi
+mv "$tmpdir/Program Files/Microsoft/VSSSDK72/inc" inc
+rm -rf $tmpdir vsssdk.msi
+exit 0