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

2013-05-24 Thread Laszlo Ersek
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



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

2013-05-24 Thread Laszlo Ersek
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 pbonz...@redhat.com
 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@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 000..5877137
 --- /dev/null
 +++ b/scripts/extract-vsssdk-headers
 @@ -0,0 +1,25 @@
 +#! /bin/bash
 +
 +# extract-vsssdk-headers
 +# Author: Paolo Bonzini pbonz...@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



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

2013-05-24 Thread Eric Blake
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]).

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



signature.asc
Description: OpenPGP digital signature


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

2013-05-24 Thread Laszlo Ersek
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.




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

2013-05-21 Thread Tomoki Sekiyama
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 pbonz...@redhat.com
Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@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 000..5877137
--- /dev/null
+++ b/scripts/extract-vsssdk-headers
@@ -0,0 +1,25 @@
+#! /bin/bash
+
+# extract-vsssdk-headers
+# Author: Paolo Bonzini pbonz...@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




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

2013-05-21 Thread 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 pbonz...@redhat.com
 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@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 pbonz...@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?

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



signature.asc
Description: OpenPGP digital signature


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

2013-05-21 Thread Tomoki Sekiyama
On 5/21/13 12:48 , Eric Blake ebl...@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 pbonz...@redhat.com
 Signed-off-by: Tomoki Sekiyama tomoki.sekiy...@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 pbonz...@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,
-- 
Tomoki Sekiyama