Re: [hatari-devel] ACSI: READ/WRITE (10) incorrectly evaluate block number

[ Thread Index | Date Index | More lists.tuxfamily.org/hatari-devel Archives ]


Am Fri, 25 Oct 2013 18:14:56 +0200
schrieb Uwe Seimet <Uwe.Seimet@xxxxxxxxx>:

> Hi all,
> 
> When reading/writing sectors with READ (10) and WRITE (10) the block
> number is not correctly evaluated. This results in the drive image
> being damaged when writing with WRITE (10). At least images > 1 GB
> are affected.
> 
> This is from hdc.c:
> 
> static unsigned long HDC_GetOffset(void)
> {
> 	/* offset = logical block address * 512 */
> 	return HDCCommand.opcode < 0x20?
> 		// class 0
> 		(HDC_ReadInt24(HDCCommand.command, 1) & 0x1FFFFF) <<
> 9 : // class 1
> 		HDC_ReadInt32(HDCCommand.command, 2) << 9;
> }
> 
> I think that the last line is wrong because the 32 bit block number is
> shifted to the left for no apparent reason. This should be:
> 
>         return HDCCommand.opcode < 0x20?
>                 // class 0
>                 (HDC_ReadInt24(HDCCommand.command, 1) & 0x1FFFFF) <<
> 9 : // class 1
>                 HDC_ReadInt32(HDCCommand.command, 2);
> 
> I stumbled upon this problem when running a small test program that
> compares the data read by various SCSI READ commands. Before applying
> this change a mismatch was reported, after applying the patch the same
> data are returned for both READ (6) and READ (10).

No, I think the problem is something else - as far as I understand that
code, HDC_GetOffset is supposed to return the offset in bytes - that's
why it shifts the value by 9 bit, it "multiplies" it with the sector
size of 512.

I think you rather run into integer overflow problems here.
nLastBlockAddr is only a 32-bit value, and fseek also does not support big
files ... so you certainly run into problems when your HD image is
bigger than 2 GB. Did you also tried with images > 1 GB but < 2 GB? I'd
expect that at least all images < 2 GB would work.

Anyway, if I find some spare time at the weekend, I'll try to clean
up that mess, e.g. by using fseeko() instead of fseek() everywhere...

 Thomas



Mail converted by MHonArc 2.6.19+ http://listengine.tuxfamily.org/