On Fri, Jul 25, 2008 at 01:42:52AM -0700, David Miller wrote:

From: Ben Dooks <ben-linux@fluff.org>
Date: Fri, 25 Jul 2008 09:34:48 +0100

quoted

On Thu, Jul 24, 2008 at 11:38:31PM -0700, David Miller wrote:

quoted

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 07da5fb..8aae917 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c

@@ -510,10 +510,8 @@ void ide_fixstring (u8 *s, const int bytecount, const int byteswap)
 
 	if (byteswap) {
 		/* convert from big-endian to host byte order */
-		for (p = end ; p != s;) {
-			unsigned short *pp = (unsigned short *) (p -= 2);
-			*pp = ntohs(*pp);
-		}
+		for (p = end ; p != s;)
+			be16_to_cpus((u16 *)(p -= 2));

personally, i would much prefer to see the loop being less evil
like:

	for (p = s; p < end; p += 2)
		be16_to_cpus((u16 *)p);

is there an architecture/compiler combo which really makes this
evil worthwile? on arm (gcc 4.2), both evaluate to the same number of
instructions.

Regardless of what we want to do with this ugly loop, the endianness
macros should be fixed to consistently evaluate their arguments
once just like real function calls do.

Yes, I wasn't saying the macro fixes are not worthwile. I would also
like to see the loop being fixed to not perpetrate this nasty code
any further.

-- 
Ben

Q:      What's a light-year?
A:      One-third less calories than a regular year.