Re: Updated PedUnit patch

[Andrew Clausen]

On Wed, Jun 29, 2005 at 11:22:20AM +0200, address@hidden wrote:
> On Wed, Jun 29, 2005 at 08:43:56AM +1000, Andrew Clausen wrote:
> > Have you run any basic tests on your code?  If it basically works, feel
> > free to commit what you have to cvs.  We can always change things
> > afterwards...
> Can we set a release date?

If you like.  How about the weekend?

> > Oops, you shouldn't free *sector.  Only *range.
> Why?

*sector isn't a pointer.  Did you try compiling?!

> > You should always be suspicious of a piece of code you don't understand!
> I am - that's why I thought about it. And division by zero looks like a pretty
> good rationale to me.

Not to me.  When is a device going to have a length of 0?

> > IIRC, this was to make formatting and inputting consistent.  I now think
> > this code is bogus, so we should just kill it.
> Can you be more specific (on both sentences)?

Well, when reading in (with ped_unit_parse), 99% might refer to a
different sector than when writing out (with ped_unit_format).

> > > > Do we need the braces?
> > > I'm afraid yes.
> > Why?
> gcc complains; you cannot declare variables without the block.

Weird.  We could stick the variables at the top...

> > This is still broken.  I would write it something like this:
> > 
> > > +     if (!ped_geometry_test_sector_inside (*range, *sector)) {
> > > +         ped_exception_throw (
> > > +                 PED_EXCEPTION_ERROR, PED_EXCEPTION_CANCEL,
> > > +                 _("The location %s is outside of the device %s."),
> > > +                 str, dev->path);
> > > +         *sector = 0;
> > > +         ped_free (*range);
> > > +         *range = NULL;
> > > +         return 0;
> > > +     }
> Why do you reset *range and *sector?

Defensive programming.  If a caller erroneously accesses *range, then we
want Parted to crash spectacularly and immediately, so we can trace the bug.

> > I notice you have started putting opening braces on their own line.
> Not exactly. I mimic coding style when I change things, but when I
> write larger parts myself I tend to use my own.
> However, I can adapt the Linux style if you want me to.

I think it's good to be consistent within a single source file.

> > Why the brackets around the return value?
> I insert brackets whenever any calculations are involved.

I think it's harder to read, but everyone's different...

Cheers,
Andrew