Re: Updated PedUnit patch

[leslie . polzer]

Hello Andrew,

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?

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

> 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.

> 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)?

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

> 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?

> 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.

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

> Bad formatting here ^
[...]
> > +   PedGeometry     *range_start, *range_end;
> >     PedGeometry             new_geom;
> 
> Inconsistent formatting.
Thanks for spotting those!


Regards,

Leslie

pgpKnDtyvhTjO.pgp
Description: PGP signature