[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: Updated PedUnit patch
From: |
Andrew Clausen |
Subject: |
Re: Updated PedUnit patch |
Date: |
Wed, 29 Jun 2005 23:49:45 +1000 |
User-agent: |
Mutt/1.5.6+20040907i |
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
- 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, 2005/06/29
- Re: Updated PedUnit patch,
Andrew Clausen <=
- 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