Re: Updated PedUnit patch

[leslie . polzer]

On Tue, Jun 28, 2005 at 08:03:22PM +1000, Andrew Clausen wrote:
> 
> I see you included correctly it in your patch... it's needed to
> test if the user type something a long way outside of the disk,
> beyond the error in the unit size.
I see.

> > I tried to put the local variable stuff into a separate function,
> 
> I'm not sure what you mean by "local variable stuff".
The locals you proposed to remedy the line length problems.
This stuff is now in the anonymous block.

> You also need to free *sector and *range, and set them to NULL...
Fixed. I hope I never forget to free things again when throwing exceptions.

> Come to think of it, why is there any rounding here at all?
> 
> (That is, why is there a 0.5 in there?)
I didn't put it in, so I don't know for sure -- but I also pondered about it
and came to the conclusion that it is a ward against division by zero.

> > +ped_unit_get_size (PedDevice* dev, PedUnit unit)
> > +{
> > +    switch (unit)
> > +    {
> > +   case PED_UNIT_SECTOR:
> > +       return PED_SECTOR_SIZE;
> > +
> > +   case PED_UNIT_BYTE:
> > +       return 1;
> > +
> > +   case PED_UNIT_KILOBYTE:
> > +       return PED_KILOBYTE_SIZE;
> 
> Any objections to this instead:
> 
> > +   case PED_UNIT_SECTOR:   return PED_SECTOR_SIZE;
> > +   case PED_UNIT_BYTE:     return 1;
> > +   case PED_UNIT_KILOBYTE: return PED_KILOBYTE_SIZE;
That's fine by me.

> Do we need the braces?
I'm afraid yes.

> Also, this still spills of 80 characters.  Perhaps define cyl_size at
> the top?
I suggest 

PedSector cyl_size = dev->bios_geom.heads
                     * dev->bios_geom.sectors;

instead. This way we save the memory management when we do not need this 
variable.


Regards and thanks,

Leslie

pedunit.patch
Description: Text document