mirror of https://lore.kernel.org/lkml/
 help / color / mirror / Atom feed
From: Linus Torvalds <torvalds@linux-foundation.org>
To: Ben Dooks <ben-linux@fluff.org>
Cc: David Miller <davem@davemloft.net>,
	bzolnier@gmail.com, harvey.harrison@gmail.com,
	linux-ide@kernel.org, linux-kernel@vger.kernel.org
Subject: Re: recent IDE regression
Date: Fri, 25 Jul 2008 09:37:25 -0700 (PDT)	[thread overview]
Message-ID: <alpine.LFD.1.10.0807250930190.5249@nehalem.linux-foundation.org> (raw)
In-Reply-To: <20080725083448.GE8301@fluff.org.uk>



On Fri, 25 Jul 2008, Ben Dooks wrote:
> 
> 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);

Well, in this case, the code actually depends on 'p' being back at the 
start of the buffer by the end of it all, so it would need some more 
changes than that. 

But yes, I applied David's patch, but I _also_ suspect that we would be 
better off without code that does horrid things like casts and assignments 
inside the function arguments.

So it would be nice to re-code that loop to be more readable. But due to 
the reliance of 'p' being 's' after the loop, the minimal patch would be 
something like the appended.

Bartlomiej - take this or not, I'm not going to commit it - I haven't 
tested it, nor do I even have any machines that would trigger it. So this 
is more a "maybe something like this" than anything else.

		Linus

---
 drivers/ide/ide-iops.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/drivers/ide/ide-iops.c b/drivers/ide/ide-iops.c
index 8aae917..ae03151 100644
--- a/drivers/ide/ide-iops.c
+++ b/drivers/ide/ide-iops.c
@@ -506,14 +506,16 @@ void ide_fix_driveid (struct hd_driveid *id)
 
 void ide_fixstring (u8 *s, const int bytecount, const int byteswap)
 {
-	u8 *p = s, *end = &s[bytecount & ~1]; /* bytecount must be even */
+	u8 *p, *end = &s[bytecount & ~1]; /* bytecount must be even */
 
 	if (byteswap) {
 		/* convert from big-endian to host byte order */
-		for (p = end ; p != s;)
-			be16_to_cpus((u16 *)(p -= 2));
+		for (p = s ; p != end ; p += 2)
+			be16_to_cpus((u16 *) p);
 	}
+
 	/* strip leading blanks */
+	p = s;
 	while (s != end && *s == ' ')
 		++s;
 	/* compress internal blanks and strip trailing blanks */

  parent reply	other threads:[~2008-07-25 16:41 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2008-07-25  6:38 David Miller
2008-07-25  8:34 ` Ben Dooks
2008-07-25  8:42   ` David Miller
2008-07-25  8:46     ` Ben Dooks
2008-07-25 16:37   ` Linus Torvalds [this message]
2008-07-25 22:17     ` Ben Dooks
2008-07-26 12:13     ` Bartlomiej Zolnierkiewicz
2008-07-26 12:04 ` Bartlomiej Zolnierkiewicz
2008-07-27  0:31   ` David Miller
2008-07-27 13:48     ` Bartlomiej Zolnierkiewicz

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=alpine.LFD.1.10.0807250930190.5249@nehalem.linux-foundation.org \
    --to=torvalds@linux-foundation.org \
    --cc=ben-linux@fluff.org \
    --cc=bzolnier@gmail.com \
    --cc=davem@davemloft.net \
    --cc=harvey.harrison@gmail.com \
    --cc=linux-ide@kernel.org \
    --cc=linux-kernel@vger.kernel.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox

Powered by JetHome