[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.
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
- Re: Updated PedUnit patch, Leslie Patrick Polzer, 2005/06/27
- Re: Updated PedUnit patch, Andrew Clausen, 2005/06/28
- Re: Updated PedUnit patch, leslie . polzer, 2005/06/28
- Re: Updated PedUnit patch, Andrew Clausen, 2005/06/28
- Re: Updated PedUnit patch, Andrew Clausen, 2005/06/29
- Re: Updated PedUnit patch,
leslie . polzer <=
- Re: Updated PedUnit patch, Andrew Clausen, 2005/06/29
- Re: Updated PedUnit patch, leslie . polzer, 2005/06/29
- Re: Updated PedUnit patch, Andrew Clausen, 2005/06/29
- Message not available
- Re: Updated PedUnit patch, leslie . polzer, 2005/06/29