How to fix a segmentation error (core dumped) within a function using structs

Question

Trying to initialize a function so its data members are 0 for the numerical values and "none" for the string but a segmentation error occurs for the strcpy.

This is for a code that creates a 2d array of an island of x,y points and prior to the later functionality I need to initialize the function to default (0's and none). I have tried to mess with the pointers and not using strcpy but to my understanding for updating strings, strcpy is needed.

typedef struct Hill {
  char name[20];
  int loc[2];
  double height;
  double slope;
} Hill;

Hill* setHill (Hill* hill){
  strcpy(hill->name, "none");  // error appears to be here
  hill->loc[0] = 0;
  hill->loc[1] = 0;
  hill->height = 0;
  hill->slope = 0;
  return hill;
}
int main() {
   Hill* hillsArray[5];
   int i;

// calling setHill to populate an array of 5 hills
   for(i=0; i<5; ++i) {
     setHill(hillsArray[i]);
   }

// updating hill "ada's apex" 
strcpy((hillsArray[0]->name), "Ada's Apex");
hillsArray[0]->loc[0] = 12;
hillsArray[0]->loc[1] = 9;
hillsArray[0]->height = 20;
hillsArray[0]->slope = 0.25;

// updating hill turing's top
strcpy((hillsArray[1]->name), "Turing's top");
hillsArray[1]->loc[0] = 4;
hillsArray[1]->loc[1] = 3;
hillsArray[1]->height = 20;
hillsArray[1]->slope = 0.33;

This updating of the hill repeats 3 more times for a total of 5 hills but its the same code just updating each hill with different values.

Could we see the part of the code where you allocate a hill and call setHill? If you haven't allocated a hill and are passing an uninitialized pointer, then you will get a segfault. — StereoBucket, Nov 12 '19 at 00:26
The problem isn't here. It is in the code that calls setHill(). At a guess that code doesn't have a Hill object to pass a pointer to it to setHill(). — Avi Berger, Nov 12 '19 at 00:27
a case when this could fail , is if you called setHill(&hill) , with hill being a variable allocated on the stack rather than on the heap. you didn't use malloc , did you ? :p — Amine Bensalem, Nov 12 '19 at 00:43
as already mentioned we need the rest part and possibly your Hill reference is empty — AntJavaDev, Nov 12 '19 at 00:46
Is there code elsewhere that actually allocates memory for the structs that `hillsArray` will point to ? — marcolz, Nov 12 '19 at 08:57
@marcolz No, that was actually the issue, the memory allocation. The answer below describes what I was doing wrong and I didn't notice the fault of memory allocation. — ramon23, Nov 12 '19 at 17:14

yano · Answer 1 · 2019-11-12T17:10:37.633

At least one problem, you're not allocating any memory for the Hills. Hill* hillsArray[5]; is an array of Hill pointers. They point to nowhere, so when you do hill->name, you're dereferencing a bad pointer, which is undefined behavior, which in your case is manifesting itself as a seg fault. You need to allocate memory for each Hill object, either dynamically or automatically, like this:

// dynamically (following what you have now)
int main() {
   Hill* hillsArray[5];
   int i;

// calling setHill to populate an array of 5 hills
   for(i=0; i<5; ++i) {
     hillsArray[i] = malloc(sizeof(hillsArray[0]));  // equivalent to malloc(sizeof(struct Hill));
     if (hillsArray[i] == NULL)
     {
       // in the case the malloc fails, handle the error however you want
       printf("Could not allocate memory for Hill %d\n", i);
       return 1;
     }
     setHill(hillsArray[i]);
   }
   ....
   // it's considered good practice to clean up memory you dynamically allocate,
   // although you will find differing opinions of that on SO, so up to you.
   // When your process exits, the OS will clean up any allocated memory, so
   // in this case, it's not all that important to clean it up yourself. But
   // for programs that run indefinitely, it's much more of a concern.
   for (i=0; i<5; i++)
   {
     free(hillsArray[i]);
   }
}

Or if you don't want to mess with dynamically allocating memory (I don't unless I have to), you can

// setup Hills in automatic storage instead
int main() {
   Hill hillsArray[5]; // each hill is in automatic storage, probably on the stack
   int i;

// calling setHill to populate an array of 5 hills
   for(i=0; i<5; ++i) {
     setHill(&(hillsArray[i]));  // now you must pass the address of each Hill to the setHill function
   }

   ....

   // since it's automatic storage, no need to clean it up yourself
}

How to fix a segmentation error (core dumped) within a function using structs

1 Answers1