Re: "partnew" Command Writes Wrong Ending Cylinder in MPT- sburtchin Jan 25, 2007; 4:14pm

[sburtchin]

NOTE: Adding message of sburtchin Jan 25, 2007; 4:14pm to Nabble thread.


----- Original Message ----- 
From: "adrian15" <address@hidden>
To: "sburtchin" <address@hidden>
Sent: Sunday, January 21, 2007 7:07 AM
Subject: Re: "partnew" Command Writes Wrong Ending Cylinder in MPT


> Did you receive this email?
Yes.

>
> adrian15
>
> sburtchin escribió:
>
> > I would like to propose the following code change to allow blanking of a
> > partition table entry
> > with a command like "partnew (hd0,3) 0x00 0 0".  New/Changed code is in
> > bold.
> >
> >    /* Convert a LBA address to a CHS address in the INT 13 format.  */
> >    auto void lba_to_chs (int lba, int *cl, int *ch, int *dh);
> >    void lba_to_chs (int lba, int *cl, int *ch, int *dh)
> >      {
> >        int cylinder, head, sector;
> >
> >        if (lba <= 0)
> >        {
> >          *cl = 0;
> >          *ch = 0;
> >          *dh = 0;
> >         }
> >         else
> >         {
> >          sector = lba % buf_geom.sectors + 1;
> >          head = (lba / buf_geom.sectors) % buf_geom.heads;
> >          cylinder = lba / (buf_geom.sectors * buf_geom.heads);
> >
> >          if (cylinder >= buf_geom.cylinders)
> >           cylinder = buf_geom.cylinders - 1;
> >
> >          *cl = sector | ((cylinder & 0x300) >> 2);
> >          *ch = cylinder & 0xFF;
> >          *dh = head;
> >        }
> >      }
>
> Hi sburtchin,
>
> Sorry about the late answer I've been very busy with Super Grub Disk and
> university lately.
>
> Currently grub legacy source code is frozen. Developers only work on
> grub2 which promises to be smarter and better than grub legacy.
>
> I am however working on a kind of grub legacy fork named Super Grub Disk.
>
> You have proposed us a source code change. Have you tried it yourself or
> were not you able to build a grub floppy or cdrom with the changes?
I did'nt know how.  I will have to read up on that.

> I can build a SGD cdrom with the changes if you want to and you can try
> yourself to see if it works ok or not.
That would be ideal!

> About the bug do you think that if we write:
>
>        if (cylinder >= buf_geom.cylinders)
>          cylinder = buf_geom.cylinders - 1;
>
> like this:
> // cylinders correction
> buf_geom.cylinders+=2;
> if (cylinder >= buf_geom.cylinders)
>          cylinder = buf_geom.cylinders - 1;
>
> we will fix the bug?
>
> adrian15
>
That would definitely provide a workaround to the bug on MY COMPUTER  (for
the "partnew" command) provided "buf_geom.cylinders" is not used anywhere
else (I don't think that it is, but its value may be).  "Fix" is a stronger
word implying that "buf_geom.cylinders" would be assigned the correct value
to begin with.  I still have'nt figured out how that gets assigned, or if
its value gets used anywhere else that matters.  The data in the "buf_geom"
structure seems to get passed around quite a bit (eg. the "geometry"
function reports 1021 cylinders also [here its the value of
"geom.cylinders"], if that matters?).

I suspect the source of the bug was a 'dirty' fix to some earlier bug.  A
safer approach might be:


int cylinder, head, sector, bufgeomcylinders;

 bufgeomcylinders = buf_geom.cylinders
 bufgeomcylinders+=2;
 if (cylinder >= bufgeomcylinders)
          cylinder = bufgeomcylinders - 1;


leaving the "buf_geom" structure unchanged, and avoiding the potential of
awakening that earlier bug.

There are far too many global identifiers in GRUB legacy, adding greatly to
the risk that one bug fix will cause disastrous consequences somewhere else.
I am hoping that GRUB2 development will place a strong emphasis on
identifier scope so that future bug fixes won't need a 'Band-Aid' solution
or discussions about what is safe.  The other BIG advantage of keeping
things local is that novices like me could contribute with a much quicker
learning curve.  I hope I'm not being too critical, but the more well
structured the code, the more people like me who want to contribute, can.
-- 
View this message in context: 
http://www.nabble.com/%22partnew%22-Command-Writes-Wrong-Ending-Cylinder-in-MPT-tf2599372.html#a8998361
Sent from the Grub - Bugs mailing list archive at Nabble.com.