Bug 140119

Summary:	gfxboot could be more big-endian-friendly
Product:	[openSUSE] SUSE Linux 10.1	Reporter:	Colin Watson <cjwatson>
Component:	Installation	Assignee:	Steffen Winterfeldt <snwint>
Status:	RESOLVED FIXED	QA Contact:	Klaus Kämpf <kkaempf>
Severity:	Enhancement
Priority:	P5 - None
Version:	Alpha 4
Target Milestone:	---
Hardware:	BigEndian
OS:	Linux
Whiteboard:
Found By:	Other	Services Priority:
Business Priority:		Blocker:	---
Marketing QA Status:	---	IT Deployment:	---
Attachments:	make syslinux detect byte-swapped cpio archives make gfxboot detect byte-swapped cpio archives fix little-endian assumptions in mkbootmsg

Description Colin Watson 2005-12-19 10:58:53 UTC

My normal development box is a PowerPC system, and so I'd like to do most of my gfxboot theme development for Ubuntu on that architecture, using qemu to do the testing. Unfortunately this is unnecessarily inconvenient for a couple of reasons:

 * gfxboot uses cpio's old binary format, which is endian-sensitive; even worse, on a big-endian machine there's no way to tell GNU cpio to generate little-endian binary archives, and the only way to get something that gfxboot can understand is to copy the archive to a little-endian machine and unpack and repack it.

 * mkbootmsg has various little-endian assumptions in its code, such as trying to memcpy() out of pointers to integers.

I'll attach patches in a moment that repair both of these deficiencies. The first is easily handled by small patches to syslinux and gfxboot (I suppose grub and lilo need similar patches, but I don't use gfxboot with either of those) to check for the byte-swapped cpio signature and if so byte-swap parts of the cpio header to compensate. The second needs a slightly longer patch to mkbootmsg to avoid assumptions about the architecture's integer storage format and to take care to write out the compiled file with little-endian integers no matter what the host architecture; I also included a couple of (unsigned char) casts where they're needed according to the definition of the ctype functions.

Comment 1 Colin Watson 2005-12-19 11:00:06 UTC

Created attachment 61349 [details]
make syslinux detect byte-swapped cpio archives

Comment 2 Colin Watson 2005-12-19 11:01:01 UTC

Created attachment 61350 [details]
make gfxboot detect byte-swapped cpio archives

Comment 3 Colin Watson 2005-12-19 11:01:46 UTC

Created attachment 61351 [details]
fix little-endian assumptions in mkbootmsg

Comment 4 Steffen Winterfeldt 2005-12-20 15:39:00 UTC

I'll give it a try when I'm back from vacation next year. I'm not
convinced about the unsigned char casts, though. isspace() expects an int
and won't work on multibyte-sequences anyway, so I don't see a gain in
that cast.

Comment 5 Colin Watson 2005-12-20 15:52:11 UTC

isspace() does take an int, yes, but the documentation says:

  These functions check whether c, which must have the value of an unsigned char or EOF, falls into a certain character class according to the current locale.

It's basically always correct in C to cast chars to unsigned char before passing them to the ctype functions; it's a common C mistake to fail to do this (http://www.greenend.org.uk/rjk/2001/02/cfu.html#ctypechar). If you rely on C's normal integer promotion rules to promote a signed char to an int, then you'll get incorrect results if you happen to pass character 255 because that's (signed char) -1 and thus will be sign-extended to (int) -1 which is EOF; furthermore, if you pass anything which is a negative signed char other than EOF, the result is undefined.

I probably should have separated that from this patch, though, since it affects Linux/i386 more than Linux/powerpc (the latter defaults to unsigned char).

Comment 6 Steffen Winterfeldt 2005-12-20 16:03:19 UTC

I fully agree with you. All I'm saying is it will not make difference in
this case.

Comment 7 Steffen Winterfeldt 2006-01-23 10:07:37 UTC

Applied big-endian patches. Will be in gfxboot-3.2.8.