[Top][All Lists]

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

Re: Updated PedUnit patch

From: leslie . polzer
Subject: Re: Updated PedUnit patch
Date: Wed, 29 Jun 2005 11:22:20 +0200
User-agent: mutt-ng devel (Linux)

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.

> 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 (
> > +                   _("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!



Attachment: pgpKnDtyvhTjO.pgp
Description: PGP signature

reply via email to

[Prev in Thread] Current Thread [Next in Thread]