[Qemu-devel] [PATCH] Fix checksum writing in signboot.sh

[Qemu-devel] [PATCH] Fix checksum writing in signboot.sh

[Alexander Graf]

The printf command takes an octal value after \, so we have to convert
our decimal representation to octal first and then write it.

This unbreaks extboot signing. Multiboot wasn't affected yet because
the checksum was < 8.

Spotted and first patch by Glauber Costa <glommer@redhat.com>.
Printf idea by Paolo Bonzini <bonzini@gnu.org>.

Signed-off-by: Alexander Graf <agraf@suse.de>
CC: Glauber Costa <glommer@redhat.com>
CC: Paolo Bonzini <bonzini@gnu.org>
CC: Jan Ondrej <ondrejj@salstar.sk>
---
 pc-bios/optionrom/signrom.sh |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
index 4322811..975b27d 100755
--- a/pc-bios/optionrom/signrom.sh
+++ b/pc-bios/optionrom/signrom.sh
@@ -39,7 +39,8 @@ done
 
 sum=$(( $sum % 256 ))
 sum=$(( 256 - $sum ))
+sum_octal=$( printf "%o" $sum )
 
 # and write the output file
 cp "$1" "$2"
-printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
+printf "\\$sum_octal" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
-- 
1.6.0.2

[Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Avi Kivity]

On 08/01/2009 12:48 PM, Alexander Graf wrote:
> The printf command takes an octal value after \, so we have to convert
> our decimal representation to octal first and then write it.
>
> This unbreaks extboot signing. Multiboot wasn't affected yet because
> the checksum was<  8.
>
> Spotted and first patch by Glauber Costa<glommer@redhat.com>.
> Printf idea by Paolo Bonzini<bonzini@gnu.org>.
>
> Signed-off-by: Alexander Graf<agraf@suse.de>
> CC: Glauber Costa<glommer@redhat.com>
> CC: Paolo Bonzini<bonzini@gnu.org>
> CC: Jan Ondrej<ondrejj@salstar.sk>
> ---
>   pc-bios/optionrom/signrom.sh |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
> index 4322811..975b27d 100755
> --- a/pc-bios/optionrom/signrom.sh
> +++ b/pc-bios/optionrom/signrom.sh
> @@ -39,7 +39,8 @@ done
>
>   sum=$(( $sum % 256 ))
>   sum=$(( 256 - $sum ))
> +sum_octal=$( printf "%o" $sum )
>
>   # and write the output file
>   cp "$1" "$2"
> -printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
> +printf "\\$sum_octal" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc 2>/dev/null
>    

While the patch is good, the code is unreadable.  Can we mandate python 
for such tricks?

    f = file(out, 'r+b')
    f.seek(size)
    f.write(chr(sum))

sh is not a sane programming language.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Filip Navara]

On Sun, Aug 2, 2009 at 12:04 PM, Avi Kivity<avi@redhat.com> wrote:
> On 08/01/2009 12:48 PM, Alexander Graf wrote:
>>
>> The printf command takes an octal value after \, so we have to convert
>> our decimal representation to octal first and then write it.
>>
>> This unbreaks extboot signing. Multiboot wasn't affected yet because
>> the checksum was<  8.
>>
>> Spotted and first patch by Glauber Costa<glommer@redhat.com>.
>> Printf idea by Paolo Bonzini<bonzini@gnu.org>.
>>
>> Signed-off-by: Alexander Graf<agraf@suse.de>
>> CC: Glauber Costa<glommer@redhat.com>
>> CC: Paolo Bonzini<bonzini@gnu.org>
>> CC: Jan Ondrej<ondrejj@salstar.sk>
>> ---
>>  pc-bios/optionrom/signrom.sh |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/pc-bios/optionrom/signrom.sh b/pc-bios/optionrom/signrom.sh
>> index 4322811..975b27d 100755
>> --- a/pc-bios/optionrom/signrom.sh
>> +++ b/pc-bios/optionrom/signrom.sh
>> @@ -39,7 +39,8 @@ done
>>
>>  sum=$(( $sum % 256 ))
>>  sum=$(( 256 - $sum ))
>> +sum_octal=$( printf "%o" $sum )
>>
>>  # and write the output file
>>  cp "$1" "$2"
>> -printf "\\$sum" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc
>> 2>/dev/null
>> +printf "\\$sum_octal" | dd of="$2" bs=1 count=1 seek=$size conv=notrunc
>> 2>/dev/null
>>
>
> While the patch is good, the code is unreadable.  Can we mandate python for
> such tricks?

No, please, no! Throwing additional tools at the problem is only going
to make it worse for Windows users. I'm not happy with using sh script
as it already added dependency on coreutils, but at least that's easy
to install. Python is a nightmare compared to that.

BTW, for years in ReactOS we had a way to build host tools with host
CC and these tools were written in plain ordinary C. This worked great
for both Windows and Linux builds and also for cross-compiling.

Best regards,
Filip Navara

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Avi Kivity]

On 08/02/2009 01:25 PM, Filip Navara wrote:
>> While the patch is good, the code is unreadable.  Can we mandate python for
>> such tricks?
>>      
>
> No, please, no! Throwing additional tools at the problem is only going
> to make it worse for Windows users. I'm not happy with using sh script
> as it already added dependency on coreutils, but at least that's easy
> to install. Python is a nightmare compared to that.
>    

Is Python really so difficult to install under Windows?  How many times 
do you have to click 'Next'?

Note that Windows users can usually use prebuilt binaries, so the 'Next' 
nightmare only affects a small number of Windows developers.

> BTW, for years in ReactOS we had a way to build host tools with host
> CC and these tools were written in plain ordinary C. This worked great
> for both Windows and Linux builds and also for cross-compiling.
>    

But then you have to write those tools in C, which is annoying.

-- 
error compiling committee.c: too many arguments to function

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Alexander Graf]

On 02.08.2009, at 13:15, Avi Kivity <avi@redhat.com> wrote:

> On 08/02/2009 01:25 PM, Filip Navara wrote:
>>> While the patch is good, the code is unreadable.  Can we mandate  
>>> python for
>>> such tricks?
>>>
>>
>> No, please, no! Throwing additional tools at the problem is only  
>> going
>> to make it worse for Windows users. I'm not happy with using sh  
>> script
>> as it already added dependency on coreutils, but at least that's easy
>> to install. Python is a nightmare compared to that.
>>
>
> Is Python really so difficult to install under Windows?  How many  
> times do you have to click 'Next'?
>
> Note that Windows users can usually use prebuilt binaries, so the  
> 'Next' nightmare only affects a small number of Windows developers.
>
>> BTW, for years in ReactOS we had a way to build host tools with host
>> CC and these tools were written in plain ordinary C. This worked  
>> great
>> for both Windows and Linux builds and also for cross-compiling.
>>
>
> But then you have to write those tools in C, which is annoying.

Right. In fact we just switched from C to sh for portability reasons.

I really think we should just make the current code work as is and be  
done. The script is pretty small and really readable IMHO.


Alex

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Avi Kivity]

On 08/02/2009 02:58 PM, Alexander Graf wrote:
> I really think we should just make the current code work as is and be 
> done. The script is pretty small and really readable IMHO.

Personally, I've stopped writing even one-liners in bash.  I'm offended 
by languages that make it nearly impossible to write correct programs.

-- 
error compiling committee.c: too many arguments to function

[Qemu-devel] Re: Re: [PATCH] Fix checksum writing in signboot.sh

[Sebastian Herbszt]

Alexander Graf wrote:
> 
> On 02.08.2009, at 13:15, Avi Kivity wrote:
>> But then you have to write those tools in C, which is annoying.
> 
> Right. In fact we just switched from C to sh for portability reasons.

I though the switch was made because Anthony didn't like running just compiled
code in the build process [1][2].

I guess the C code was most portable anyway because it didn't introduce new
dependencies.

[1] http://lists.gnu.org/archive/html/qemu-devel/2009-07/msg00051.html
[2] http://lists.gnu.org/archive/html/qemu-devel/2009-07/msg00103.html

- Sebastian

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Anthony Liguori]

Alexander Graf wrote:
>
> On 02.08.2009, at 13:15, Avi Kivity <avi@redhat.com> wrote:
>
>> On 08/02/2009 01:25 PM, Filip Navara wrote:
>>>> While the patch is good, the code is unreadable.  Can we mandate 
>>>> python for
>>>> such tricks?
>>>>
>>>
>>> No, please, no! Throwing additional tools at the problem is only going
>>> to make it worse for Windows users. I'm not happy with using sh script
>>> as it already added dependency on coreutils, but at least that's easy
>>> to install. Python is a nightmare compared to that.
>>>
>>
>> Is Python really so difficult to install under Windows?  How many 
>> times do you have to click 'Next'?
>>
>> Note that Windows users can usually use prebuilt binaries, so the 
>> 'Next' nightmare only affects a small number of Windows developers.
>>
>>> BTW, for years in ReactOS we had a way to build host tools with host
>>> CC and these tools were written in plain ordinary C. This worked great
>>> for both Windows and Linux builds and also for cross-compiling.
>>>
>>
>> But then you have to write those tools in C, which is annoying.
>
> Right. In fact we just switched from C to sh for portability reasons.

The problem is with cross compilers.  Our build system is based around a 
single tool chain and we only do feature probing, sanity checking, 
cflags modifications, etc. on the target tool chain.  If we build and 
run a C program using the host compiler (which is needed in order to be 
able to run the program), things get complicated quickly.

sh is preferred because it's a minimal dependency.  I would be concerned 
about perl or python for the main build because those tools aren't 
available by default for windows.  For something like a rom where we 
ship a default binary, as long as we detected the appropriate tools and 
disabled the build, I think it would be more reasonable.

> I really think we should just make the current code work as is and be 
> done. The script is pretty small and really readable IMHO.

We're going to have to revisit this for pc-bios since it depends on perl 
and it has a similar rom signing tool (biossums).  It's far more 
sophisticated though and it's currently implemented in C.  It may make 
sense to rewrite that tool in python/perl and have a single tool used 
for all of our roms.

We don't need to do this now though.

Regards,

Anthony Liguori

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Paolo Bonzini]

>> Right. In fact we just switched from C to sh for portability reasons.
>
> The problem is with cross compilers. Our build system is based around a
> single tool chain and we only do feature probing, sanity checking,
> cflags modifications, etc. on the target tool chain. If we build and run
> a C program using the host compiler (which is needed in order to be able
> to run the program), things get complicated quickly.

One hopes that you do not need feature tests for the build compiler if 
it is used for someting as simple as generating checksums.  The build 
compiler should be just "cc" or "gcc".

That said, I don't think sh is a big problem.

Paolo

Re: [Qemu-devel] Re: [PATCH] Fix checksum writing in signboot.sh

[Avi Kivity]

On 08/03/2009 05:30 AM, Anthony Liguori wrote:

>
> sh is preferred because it's a minimal dependency.  I would be 
> concerned about perl or python for the main build because those tools 
> aren't available by default for windows. 

Neither is the compiler (it isn't even available by default in Fedora on 
my installs).  I think we can trust qemu developers to be able to 
install python.  It will become even more important if we provide an IDL 
for the monitor or want to generalize hxtool.

-- 
error compiling committee.c: too many arguments to function