| Message ID | 1249120111-31757-1-git-send-email-agraf@suse.de |
|---|---|
| State | Superseded |
| Headers | show |
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.
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
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.
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
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.
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
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
>> 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
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.
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
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(-)