[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Two bugs that cancel each other in chs_get_sector and chs_to_sector
From: |
Andrew Clausen |
Subject: |
Re: Two bugs that cancel each other in chs_get_sector and chs_to_sector |
Date: |
Sat, 4 Sep 2004 11:01:34 +1000 |
User-agent: |
Mutt/1.5.5.1+cvs20040105i |
Hi Håkon,
On Sat, Sep 04, 2004 at 02:28:48AM +0200, Håkon Løvdal wrote:
> The calculation "chs->sector % 0x40 - 1;" in chs_get_sector is wrong.
> It should not subtract one here.
I think it should. I think it is stupid to count from 1. I
unilaterally decided to count from 0 (just like cylinders and heads
are).
> Besides using the modulo operator here is weird at best. The sector is
> stored in the 6 least significant bits and a binary and operator
> together with a proper bitmask is the Correct(TM) way to extract it...
You convinced me here.
> Correspondingly there is a missing one subtraction in
> ped_disk_print. With my correction the code now matches the
> normal translation formula "c*H*S + h*S + (s-1)" (as mentioned at
> http://www.win.tue.nl/~aeb/linux/Large-Disk-3.html for instance).
I think it is a stupid formula (that matches the stupid situation).
> So the two functions chs_get_sector and chs_to_sector are very
> straight forward. However the chs_get_sector function is also used in
> probe_partition_for_geom and I updated correspondingly there, but to be
> honest I did not quite understand the algorithm/formula. Could someone
> enlighten me? Adding some printfs gave me the following for my disk:
>
> c=0, h=1, s=1, a=63, a_=63, C=1019, H=15, S=63, A=1028159, A_=1028097
> (A_ * h - a_ * H) = 1027152
> (a_ * C - A_ * c) = 64197
> (A_ * h - a_ * H) / (a_ * C - A_ * c) = 16.000000
It's just a bit of algebra to compute bios_geom. You have two
equations, and you solve for heads and sectors.
I just added these assertions at the end:
PED_ASSERT ((c * heads + h) * sectors + s == a, return 0);
PED_ASSERT ((C * heads + H) * sectors + S == A, return 0);
c, h, s, a, C, H, S, A are the known variables.
heads, sectors are the unknown variables.
If we have two indepedent equations, we can solve for heads and sectors...
> Also I replaced a few zeros with the corresponding enum value
Good point.
> and replaced all usage of 1022 with 1023; it is
> very close to Numeric Literals Coding Obfuscation, see
> http://www.web-hits.org/txt/codingunmaintainable.html (and in fact
> chs_to_sector used to give up one cylinder too early).
Fair enough.
> Maybe the last if test in chs_to_sector should be "PED_ASSERT(s ==
> 0, return 0);" instead?
Assertions are for bugs in Parted. This is an error.
Thanks for your comments.
Andrew