PDA

View Full Version : Subtracting two numbers gives EXC_BAD_ACCESS

blueeye
Nov 2, 2009, 10:11 AM
Hi MacRumours,

I'm writing an iPhone app which uses the accelerometer to control another external application (just for some context).

In order to calculate the difference between the previous accelerometer value and this one I use the following two lines (I only need X and Y):

float diffx = ([acceleration x] - [prevAccel x])*100;
float diffy = ([acceleration y] - [prevAccel y])*100;

This works most of the time, but sometimes (either randomly or when the difference becomes large, mostly the latter) I get EXC_BAD_ACCESS on the first of the two lines above.
In GDB I tried the following:

(gdb) p acceleration
\$3 = (UIAcceleration *) 0x128500
(gdb) p acceleration.x
\$4 = 0.869384765625
(gdb) p acceleration.y
\$5 = -0.036224365234375
(gdb) p acceleration.z
\$6 = -0.543365478515625
(gdb) p prevAccel.x
\$7 = 4.6495020515960297e-315
(gdb) p prevAccel.y
\$8 = 1.7286328172809227e-38
(gdb) p prevAccel.z
\$9 = 0
(gdb) p (acceleration.x - prevAccel.x)
\$10 = 0.869384765625

after I got the EXC_BAD_ACCESS code and as you can see, the sum works...

I then tried the following:

(gdb) continue
Continuing.

0x31ec3ebc in objc_msgSend ()
(gdb) x/s \$r1
0x34370cc4 <__PRETTY_FUNCTION__.55878+13008>: "x"

I'm not entirely sure what to do here since it works most of the time. Any ideas would be much appreciated.

dejo
Nov 2, 2009, 10:13 AM
How are you retaining prevAccel?

blueeye
Nov 2, 2009, 10:18 AM
How are you retaining prevAccel?

It's not retained explicitly. I tried copying the acceleration as follows:

if(prevAccel == nil) {
prevAccel = [acceleration copy];
} else {
...

but it didn't work so I've just got

prevAccel = acceleration;

robbieduncan
Nov 2, 2009, 10:23 AM
It's not retained explicitly. I tried copying the acceleration as follows:

if(prevAccel == nil) {
prevAccel = [acceleration copy];
} else {
...

but it didn't work so I've just got

prevAccel = acceleration;

So if you don't retain it and it's autoreleased what happens to it at the end of the current runloop? Normally it'll get released and next time in you'll access memory you no longer own and bang! Sometimes you'll get lucky, other times you won't.

blueeye
Nov 2, 2009, 10:24 AM
How are you retaining prevAccel?

Fabulous, I think I've fixed it by assigning prevAccel as follows:

prevAccel = [acceleration retain];

Thanks for pointing me in the right direction :)

robbieduncan
Nov 2, 2009, 10:25 AM
Fabulous, I think I've fixed it by assigning prevAccel as follows:

prevAccel = [acceleration retain];

Thanks for pointing me in the right direction :)

Remember to release it after you are done with it otherwise you are leaking memory...

blueeye
Nov 2, 2009, 10:25 AM
So if you don't retain it and it's autoreleased what happens to it at the end of the current runloop? Normally it'll get released and next time in you'll access memory you no longer own and bang! Sometimes you'll get lucky, other times you won't.

I guess it was that "Sometimes you'll get lucky bit" that threw me off the scent. I'm still relatively new to the concept of memory management.
Thanks for the help :)

Remember to release it after you are done with it otherwise you are leaking memory...

Done. Thanks again. I would have missed that otherwise.

dejo
Nov 2, 2009, 10:29 AM
Remember to release it after you are done with it otherwise you are leaking memory...
Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

blueeye
Nov 2, 2009, 10:34 AM
Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

I released before reassigning and it hasn't broken but I'm not entirely sure if it's correct either. It makes sense (to me) to do it that way but I'm hardly an expert memory manager.

PhoneyDeveloper
Nov 2, 2009, 10:53 AM
Few points.

I haven't used the accelerometer but a quick look at the header shows that it doesn't implement the NSCopying protocol so it won't copy correctly. This is arguably a bug but at any rate you can't expect the copy method to work correctly.

You probably want to add a retain property for your prevAccel ivar. That will make your memory management easier.

self.prevAccel = acceleration;

will retain the new value and release the old value.

When you see crashes in objc_msgSend they usually mean that you've tried to access an ivar where you failed to retain it.

robbieduncan
Nov 2, 2009, 11:02 AM
Does that include releasing it before reassigning it? (Sometimes memory management confuses even me.)

Yes. Otherwise the object you retained and failed to release will hang around until the app quites. If you retain an object you must release it. Assigning a new object to the pointer does not release the original object you retained...

blueeye
Nov 2, 2009, 11:03 AM
Few points.

I haven't used the accelerometer but a quick look at the header shows that it doesn't implement the NSCopying protocol so it won't copy correctly. This is arguably a bug but at any rate you can't expect the copy method to work correctly.

You probably want to add a retain property for your prevAccel ivar. That will make your memory management easier.

self.prevAccel = acceleration;

will retain the new value and release the old value.

When you see crashes in objc_msgSend they usually mean that you've tried to access an ivar where you failed to retain it.

As it stands I've simply assigned it to [acceleration retain], although prevAccel is assigned as (nonatomic, retain).

dejo
Nov 2, 2009, 11:36 AM
As it stands I've simply assigned it to [acceleration retain], although prevAccel is assigned as (nonatomic, retain).
If prevAccel is already a property with (nonatomic, retain), then just assigning it
prevAccel = [acceleration retain]; is not taking advantage of the property you have setup. As PhoneyDeveloper has suggested, you should be using self.prevAccel = acceleration; instead.

blueeye
Nov 2, 2009, 01:02 PM
If prevAccel is already a property with (nonatomic, retain), then just assigning it
prevAccel = [acceleration retain]; is not taking advantage of the property you have setup. As PhoneyDeveloper has suggested, you should be using self.prevAccel = acceleration; instead.

Ahh! I see. I'd misunderstood and thought that writing

prevAccel

was a convenience method of accessing

self.prevAccel

Thanks for clearing this up.

dejo
Nov 2, 2009, 07:33 PM
Ahh! I see. I'd misunderstood and thought that writing

prevAccel

was a convenience method of accessing

self.prevAccel

Thanks for clearing this up.
Using just prevAccel accesses the instance variable directly and doesn't use the getters or setters created via the @synthesize part of your property definition.

blueeye
Nov 3, 2009, 04:08 AM
Using just prevAccel accesses the instance variable directly and doesn't use the getters or setters created via the @synthesize part of your property definition.

I'll definitely keep that in mind then.
When I try using

self.prevAccel = acceleration;

Do I still have to using [acceleration retain] or is it sufficient that prevAccel has retain set?

robbieduncan
Nov 3, 2009, 04:14 AM
I'll definitely keep that in mind then.
When I try using

self.prevAccel = acceleration;

Do I still have to using [acceleration retain] or is it sufficient that prevAccel has retain set?

You do not.

self.prevAccel = acceleration;

is equivalent to

[self setPrevAccel:acceleration];

The synthesized code for setPrevAccel will look something like

-(void) setPrevAccel:(UIAcceleration) newValue
{
if (prevAccel!= newValue) {

[prevAccel release];

prevAccel= [newValue retain];

}
}

As you can see this releases any existing value set for the property and retains the new value for you (this assumes you declared the property as retain)

blueeye
Nov 3, 2009, 04:16 AM
@robbieduncan:

Okay, that's brilliant. Thanks for explaining that to me.