Re: [PATCH 0/3] ide: Fix use of paired device

[Benjamin Herrenschmidt <benh@kernel.crashing.org>]

On Thu, 2007-10-18 at 15:54 +0400, Sergei Shtylyov wrote:
> Hello.
> 
> Benjamin Herrenschmidt wrote:
> 
> > At least 2 drivers (siimage and cs5535) have a bug where they use
> > the construct:
> 
> > 	ide_drive_t *pair	= &hwif->drives[drive->dn ^ 1];
> 
> > To access the other drive in a master/slave pair. This is bogus
> > because drive->dn is not the unit number, but the global drive
> > number, thus can be 2 & 3 for ide1, 4 & 5 for ide2 etc...
> 
>     Huh? With drive->dn calculated as:
> 
>          for (unit = 0; unit < MAX_DRIVES; ++unit) {
>                  ide_drive_t *drive = &hwif->drives[unit];
>                  drive->dn = (hwif->channel ? 2 : 0) + unit;
> 
> (with MAX_DRIVES always being 2) how comes it may be 4 or 5?!
> 
> > Please apply to 2.6.24 if no objection.
> 
>     I object. :-)

allright, I should have read the code more closely :-)

I saw drive->dn being 2 on ide1, so the code misbehaving because of
that, and thus drawn an incorrect conclusion.

Still.. the code is bogus and having an helper to get the paired device
makes sense, so all we need to do is fix the implementation of that
helper. The good thing is that I think my patch is still correct, though
the comment is not :-)

Ben.


-
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/