1

I've got a c-array of CGPoints in a struct. I need to replace this array when another CGPoint is added. I'd swear I'm doing this right and it seems to work fine a few times but eventually I'll get a EXC_BAD_ACCESS. What am I missing?

Here's the struct, which I've truncated to remove a lot of items that don't pertain.

typedef struct{
    CGPoint **focalPoints;
    NSUInteger focalPointCount;
    CGRect boundingRect;
}FocalPoints;

Here's how I initialize it:

CGPoint *fPoints = (CGPoint *)malloc(sizeof(CGPoint));
FocalPoints focalInfo = {&fPoints, 0, rect};

Note that focalInfo is passed by reference to another function, like so: anotherFunction(&focalInfo).

Now here's the function that replaces the Points array with a new one:

void AddFocalPoint (CGPoint focalPoint, FocalPoints *focal){
    if (focalPoint.x == CGFLOAT_MAX) return;
    if (!CGRectContainsPoint(focal->boundingRect, focalPoint)) return;
    int origCount = focal->focalPointCount;
    int newCount = origCount + 1;
    CGPoint *newPoints = (CGPoint *) malloc((newCount) * sizeof(CGPoint));
    for (int i = 0; i < newCount; i++)
        newPoints[i] = (i < origCount) ? *focal->focalPoints[i] : focalPoint; //error occurs here
    free(*focal->focalPoints);
    *focal->focalPoints = newPoints;
    focal->focalPointCount = newCount;
}

The EXC_BAD_ACCESS error occurs in the above code on line 8: newPoints[i] = (i < origCount) ? *focal->focalPoints[i] : focalPoint;. So what exactly am I doing wrong?

Improve this question
5
  • I'm not sure entirely, but I think newPoints[i] is a CGPoint, whereas *focal->focalPoints[i] is a CGPoint pointer... Commented Mar 19, 2012 at 20:45
  • 1
    Have you considered using a linked list or even NSMutableArray/NSValue so that you're not allocating, copying, and freeing the whole thing every time you want to add a point? Commented Mar 19, 2012 at 20:49
  • The original malloc of fPoints isn't technically used. But I allocate the space because the AddFocalPoint function will call free on it, and I don't want to free space that hasn't been malloc'd. &fPoints add an additional pointer to fPoints. I do this because I believe that's the only way to replace c-arrays. Commented Mar 19, 2012 at 20:49
  • 2
    "The original malloc() of fPoints isn't technically used": You don't need to allocate that, then -- as long as you initialize it to NULL, you'll be fine. free(NULL) is a perfectly safe no-op. Commented Mar 19, 2012 at 20:53
  • lol...oh yeah, I'd forgotten about that. Thanks, it's been a little bit since I messed with c-pointers. Commented Mar 19, 2012 at 20:54

3 Answers 3

Reset to default
3

This is a bit of a long shot, but maybe there's an issue with operator priority in *focal->focalPoints[i]. Have you try adding parentheses according to what you are trying to achieve ?

Improve this answer
Sign up to request clarification or add additional context in comments.

3 Comments

What do you mean by parenthesis? Where would I put them?
in *focal->focalPoints[i], I don't know wether it's *(focal->focalPoints[i]), *(focal->focalPoints)[i], (*focal)->focalPoints[i].. I think you see what I'm trying to say here
Well, what do you know. You were right. I added a line CGPoint *focalPoints = *focal->focalPoints; and I accessed it like so: newPoints[i] = (i < origCount) ? focalPoints[i] : focalPoint;
2

I believe the issue comes with where GCPoint *fPoints allocated as &fPoints evaluates to an address of that ... which is no longer valid once the function exits.

(The data to which it points was allocated fine with malloc.)

Improve this answer

2 Comments

The struct is not accessed after the function exits. I pass the struct by reference to another function. When that function returns, I take the data modified in the struct and pass it to another function (all still in the function that created the struct). Then the function exits, the struct is destroyed. It is accessed in no other place.
Well, there goes my idea. Good luck sorting it out :(
1

Aside from the suggestion I made in a comment, of using a linked list/NSMutableArray, my other suggestion would be that you use realloc() instead of constantly using malloc(), copying by hand, and then free()ing the old allocation.

void * realloc(void *ptr, size_t size);
The realloc() function tries to change the size of the allocation pointed to by ptr to size, and returns ptr. If there is not enough room to enlarge the memory allocation pointed to by ptr, realloc() creates a new allocation, copies as much of the old data pointed to by ptr as will fit to the new allocation, frees the old allocation, and returns a pointer to the allocated memory.

This is pretty much exactly what you are doing, but you can let the library handle it for you.

(May I also humbly suggest using the word "focal" slightly less to name variables in your function?) (Also also, I'm not really clear on why focalPoints in your struct is a pointer-to-pointer. You just want an array of structs -- a single pointer should be fine.)

Consider the following (somewhat extensive) rewrite; hope that it's helpful in some way.

typedef struct{
    CGPoint *points;    // Single pointer
    NSUInteger count;
    CGRect boundingRect;
} FocalPoints;

// Renamed to match Apple's style, like e.g. CGRectIntersectsRect()
void FocalPointsAddPoint (FocalPoints *, CGPoint);

void FocalPointsAddPoint (FocalPoints *f, CGPoint thePoint){
    if (thePoint.x == CGFLOAT_MAX) return;
    if (!CGRectContainsPoint(f->boundingRect, thePoint)) return;
    NSUInteger origCount = f->count;    // |count| is typed as NSUInteger; |origCount|
    NSUInteger newCount = origCount + 1;    // and |newCount| should be consistent
    // Greatly simplified by using realloc()
    f->points = (CGPoint *) realloc(f->points, newCount * sizeof(CGPoint));
    (f->points)[newCount-1] = thePoint;
    f->count = newCount;
}

int main(int argc, const char * argv[])
{

    @autoreleasepool {
        // Just for testing; any point should be inside this rect
        CGRect maxRect = CGRectMake(0, 0, CGFLOAT_MAX, CGFLOAT_MAX);
        // Can initialize |points| to NULL; both realloc() and free() know what to do
        FocalPoints fp = (FocalPoints){NULL, 0, maxRect};
        int i;
        for( i = 0; i < 10; i++ ){
            FocalPointsAddPoint(&fp, CGPointMake(arc4random() % 100, arc4random() % 100));
            NSLog(@"%@", NSStringFromPoint(fp.points[i]));
        }

    }
    return 0;
}
Improve this answer

3 Comments

Excellent, thank you. I'm sorry I've already accepted another answer, but I did up-vote yours. First, I can't use obj-c objects because I'm using ARC, which won't allow it, so NSMutableArray was out. I didn't know about realloc which really did simplify stuff, thanks! The reason I was using a double-pointer for my c-array was I simply couldn't replace the array with the new one. No matter how I tried, when I exited the function the struct retained a pointer to the old array. I found the answer for that here: stackoverflow.com/questions/1106957/….
Glad it was helpful. Don't worry about the accept; I've got more useless rep than I know what to do with anyways. There should be a way to get an object into a struct even with ARC (I think __unsafe_unretained might be all you need) but it may not be worth the headache.
Yeah, that would probably work but I got this working right now so I'm not going to mess with it. Besides, I enjoyed getting back to some c.

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.