Segmentation fault with struct array

Question

Trying to calculate the left most point in an Array of Points, the program blows up on me (segmentation fault (core dump) error).

Here's the interface:

//points.h
#define MAX_POINTS 100

struct Point {
   char label;
   int x;
   int y;
};

int leftmostPoint(struct Point points[], int numPoints);

Here's the leftmostPoint implementation:

//points.c
//get the point with the smallest x value
int leftmostPoint(struct Point points[], int numPoints) {
   int smallestX = points[0].x; //assume first point is smallest
   int index;
   for (int i = 1; i < numPoints; i++) {
      if (points[i].x < smallestX) {
         smallestX = points[i].x;
         index = i;
      }
   }
   return points[index];
 }

Here's where the magic happens:

//magic.c
struct Point points[MAX_POINTS];
//build array via standard input (this works, tested by printing the points)
//only 5 points were added in
displayPoint(points[0]); //works
displayPoint(points[4]); //works

struct Point hull;

hull = leftmostPoint(points, numPoints); //this is where the program blows up

I am pretty sure it's an issue of sending pointers and not actual copies of the array (curse c!!), my question is where is the issue exactly and how can I go about fixing it?

Sorry Jonathan, that was a typo when I was writing up the code here in Stack Overflow! My code reflects the appropriate return, leftmostPoint() does indeed return a struct Point — Talen Kylon
– Talen Kylon, Commented Oct 7, 2013 at 1:40
When you call leftmostPoint, you pass numPoints. Where is that variable defined? — Yoctohan
– Yoctohan, Commented Oct 7, 2013 at 1:41
Sorry, numPoints is defined earlier in the main after I read how many points the user has inputted. In this example, numPoints is 5. — Talen Kylon
– Talen Kylon, Commented Oct 7, 2013 at 1:43

Community · Accepted Answer · 2017-05-23 12:10:56Z

4

In the original version of the code, your function leftmostPoint() was supposed to return an int but you return a struct Point. The compiler should be complaining about this. (The code has since been updated to return a struct Point.)

The invocation:

struct Point hull = leftmostPoint(points, numPoints);

indicates the problem is in the declaration of leftmostPoint(), which should be returning a struct Point instead of an int.

So, fix either by:

struct Point (leftmostPoint(struct Point points[], int numPoints)
{
    int smallestX = points[0].x; //take the first point in the list and assume it's smallest
    int index = 0;
    for (int i= 1; i < numPoints; i++){
        if (points[i].x < smallestX){
           smallestX = points[i].x;
           index = i;
       }
    }
    return points[index];
}

Or by:

int leftmostPoint(struct Point points[], int numPoints)
{
    int smallestX = points[0].x; //take the first point in the list and assume its smallest
    int index = 0;
    for (int i= 1; i < numPoints; i++){
        if (points[i].x < smallestX){
           smallestX = points[i].x;
           index = i;
       }
    }
    return index;
}

My suspicion is that the version returning the int is more useful; you need to know which entry in the array was the left-most, rather than just the value of the entry.

You'll also note that paxdiablo set index to zero so as to avoid the possibility of returning a "random" value if the first item in the array is the one with the lowest x value.

Given that you've fixed what should have been compilation problems, the next question should be indeed be:

What is the value of numPoints in the call to the function?

You can always add printing code to a function to check that you're getting correct data:

struct Point (leftmostPoint(struct Point points[], int numPoints)
{
    int smallestX = points[0].x; //take the first point in the list and assume it's smallest
    int index = 0;
    assert(numPoints > 0);
    printf("-->> %s: numPoints = %d: index = %d, x = %d\n",
           __func__, numPoints, index, smallestX);
    for (int i= 1; i < numPoints; i++){
        if (points[i].x < smallestX){
            smallestX = points[i].x;
            index = i;
            printf("---- %s: index = %d, x = %d\n", __func__, index, smallestX);
       }
    }
    printf("<<-- %s: index = %d: x = %d\n", __func__, index, points[index].x);
    return points[index];
}

Or variants on that theme.

edited May 23, 2017 at 12:10

CommunityBot

11 silver badge

answered Oct 7, 2013 at 1:39

Jonathan Leffler

759k145 gold badges961 silver badges1.3k bronze badges

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

2 Comments

paxdiablo Over a year ago

Jonathan, I've taken the opportunity to init index before the loop since it's likely to return a random value if points[0] has the smallest x. Feel free to change my wording if it doesn't gel well with your normal prose.

Jonathan Leffler Over a year ago

@paxdiablo: thanks — I incorporated your change (or equivalent) while I was editing (including the comment about initializing index to 0).

Collectives™ on Stack Overflow

Segmentation fault with struct array

1 Answer 1

2 Comments

Your Answer

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

2 Comments

Your Answer

Sign up or log in

Post as a guest

Related