CODEWITHSUNDEEP
cpointersstructsegmentation-faultmalloc
Mark
Mark

Reputation: 61

Returning a pointer to a struct using dynamic memory allocation

I'm trying to return a pointer to a new struct I've created but I get a segmentation fault and am unsure how to do this the correct way.

This is what I'm using right now:

 typedef struct person{
  int age;
  char *name;
 }Person;

 Person *new_person(int age, const char *name){
  Person *x = malloc(sizeof(x));
  x->age = age;
  strcpy(x->name, name);
  return x;
 }

I've been tweaking with this code from tutorials/other questions I've found online. But I can't seem to figure out what I'm doing wrong, whether it's the way I allocate the memory or the way I return the pointer.

Another issue I have is deallocating any associates memory with a certain Person. Say I wanted to remove a person, and deallocate any dynamically associated memory with that person. I searched it and it said I should to use the free method. But I still get a memory leak with it. I have something set-up like this:

void kill_person(Person *x){
    free(x->name);
    free(x);
}

Should I be calling free(*x)? And should I also be calling free(x->name)?

Upvotes: 0

Views: 112

Answers (2)

Dr. Debasish Jana
Dr. Debasish Jana

Reputation: 7128

You need to allocate space for Person as well as it's pointer within i.e name. Othertwise, evenif Person gets allocated it's name portion will remain unallocated.

Option 1:

typedef struct person{
  int age;
  char *name;
 }Person;

Person *new_person(int age, const char *name){
  Person *x = (Person *) malloc(sizeof(Person));
  if (name != NULL) {
      x->name = (char *) malloc(strlen(name) + 1);
  }
  x->age = age;
  strcpy(x->name, name);
  return x;
 }
void kill_person(Person *x){
    free(x->name);
    free(x);
}
int main() {
  Person *p = new_person(28, "Neel Lohit");
  // code to use p, when done, call kill_person
  kill_person(p);
  return 0;
}

Option 2 (name portion having fixed static allocation):

#define MAX_SZ 100
typedef struct person{
  int age;
  char name[MAX_SZ+1];
 }Person;

Person *new_person(int age, const char *name){
  Person *x = (Person *) malloc(sizeof(Person));
  x->age = age;
  strcpy(x->name, name);
  return x;
 }
void kill_person(Person *x){
    free(x);
}
int main() {
  Person *p = new_person(28, "Neel Lohit");
  // code to use p, when done, call kill_person
  kill_person(p);
  return 0;
}

Upvotes: 1

Ed Swangren
Ed Swangren

Reputation: 124790

Person *x = malloc(sizeof(x));

You meant to write

Person *x = malloc(sizeof(*x));

sizeof x == sizeof(Person*), so you only allocated enough memory for a pointer. You're next segfault will (probably) be on this line: 

strcpy(x->name, name);

You never allocated x->name. A proper function would be:

Person *new_person(int age, const char *name) {
    Person *x = malloc(sizeof *x);
    x->name = malloc(strlen(name) + 1);
    x->age = age;
    strcpy(x->name, name);
    return x;
}

Upvotes: 2

Related Questions