Освобождение памяти в C

Question

У меня проблема с этой маленькой программой:

ОБНОВЛЕНО (Согласно некоторым запросам, я включил все здесь, чтобы прояснить, что я делаю. Извините, что это слишком долго): Файл Student.h:

typedef struct Student {
  char *name;
  int age;
  char *major;
    char *toString;
} *Student;

extern Student newStudent(char *name, int age, char *major);

Файл Student.c: с

har *studentToString(Student s);
static void error(char *s) {
  fprintf(stderr,"%s:%d %s\n",__FILE__,__LINE__,s);
  exit(1);
}

extern Student newStudent(char *name, int age, char *major) {
  Student s;
    if (!(s=(Student)malloc(sizeof(*s)))){
    error("out of memory");
    }
  s->name=name;
  s->age=age;
  s->major=major;
    s->toString = studentToString(s);
  return s;
}

char *studentToString(Student s) {
  const int size=3;
  char age[size+1];
  snprintf(age,size,"%d",s->age);

  char *line=newString();
  line=catString(line,"<");
  line=catString(line,s->name);
  line=catString(line," ");
  line=catString(line,age);
  line=catString(line," ");
  line=catString(line,s->major);
  line=catString(line,">");
  return line;
}

Файл Students.c:

static void error(char *s) {
  fprintf(stderr,"%s:%d %s\n",__FILE__,__LINE__,s);
  exit(1);
}

static StudentList alloc(StudentList students, Student student) {

  StudentList p;
    if (!(p=(StudentList)malloc(sizeof(*p)))){
    error("out of memory");}
  p->student=student;
  p->students=students;
  return p;
}

extern Students newStudents() {
  Students p;
    if (!(p=(Students)malloc(sizeof(*p)))){
    error("out of memory");
    }
  p->cursor=0;
  p->students=0;
  return p;
}

extern void addStudent(Students students, Student student) {
  StudentList p=students->students;
  if (!p) {
    students->students=alloc(0,student);

    return;
  }
while (p->students)
     p=p->students;
  p->students=alloc(0,student); 
    }    

extern void initStudent(Students students) {
  students->cursor=students->students;
}

extern Student currStudent(Students students) {
  StudentList cursor=students->cursor;
  if (!cursor)
    return 0;
  return cursor->student;
}

extern void nextStudent(Students students) {
  students->cursor=students->cursor->students;
}

И мой основной метод:

int main() {
  Students students=newStudents();
  addStudent(students,newStudent("Julie",22,"CS"));
  addStudent(students,newStudent("Trevor",32,"EE"));

  for (initStudent(students);
       currStudent(students);
       nextStudent(students)) {
      char *line=currStudent(students)->toString;
    printf("%s\n",line);
      free(currStudent(students));
    free(line);
  }

    free(students->students);
    free(students);
  return 0;
}

Я использую valgrind для проверки утечек памяти, и появляется следующая ошибка:

8 bytes in 1 blocks are definitely lost in loss record 1 of 1
==9520==    at 0x40054E5: malloc (vg_replace_malloc.c:149)
==9520==    by 0x8048908: alloc (Students.c:13)
==9520==    by 0x80489EB: addStudent (Students.c:42)
==9520==    by 0x804882E: main (StudentList.c:10)

Я понимаю, что мне нужно освободить память, выделенную для p в функции alloc , но где я должен вызывать free (p)? Или я что-то не так делаю? Пожалуйста, помогите!

Answer 1

Извините, это тангенциально, но вы действительно могли бы сделать многое, чтобы сделать ваш код более читабельным.

Вы создаете структуру, а затем переопределяете ее тип в качестве указателя на нее. Да, это просто напрашивается на неприятности, когда дело доходит до ремонтопригодности. Обычно плохая идея скрывать тот факт, что указатели являются указателями, потому что когда люди видят что-то вроде этого:

Students newStudents()
{
  Students p;
  // ...
  return p;
}

Соглашение

заставляет нас предполагать, что вы возвращаете структуру, которая была размещена в стеке, что было бы явно неверно. (Изменить: не обязательно «явно неверно», но расточительная копия.)

Когда вы добавляете свой malloc, все становится еще приятнее ...

Students p;
if (!(p=(Students)malloc(sizeof(*p))))
{
  error("out of memory");
}

Во-первых, как уже упоминалось, люди предполагают, что без *, Student является полной структурой в стеке. Это заставит любого, кто видит "sizeof (* p)", сделать двойной дубль. Не очевидно, что вы делаете.

И хотя сжатие назначений и сравнений в одно выражение if совершенно допустимо для C и C ++, обычно это не самое читаемое решение. Значительно улучшено:

Students* newStudents ()
{
  Students* p = (Students*) malloc (sizeof (Students));
  if (p == NULL)
  {
     // ...
  }
  // ...
  return p;
}

И людям нравится указывать, что приведение возвращаемого значения malloc не обязательно в C, но это в C ++.

Что касается вашей утечки, хорошо ... valgrind не сообщил о вашем использовании catString, но все еще довольно схематично, так как вы скрываете использование памяти. Использование snprintf - лучший идиоматичный способ создания нужной строки.

Утечка valgrind сообщает: похоже, вы просто освобождаете первый «ученический» узел в вашем списке. Вам нужно пройти его и освободить их всех, вероятно, так:

Students p = students;
while (p)
{
  Students next = p->students;
  free (p);
  p = next;
}

Answer 2

Вопрос в том, что вы будете делать, когда вы закончите с Student или StudentList и вам это больше не понадобится. Это тот момент, когда вы должны вызывать free() для всех выделенных вещей в этой структуре.

Вероятно, вы захотите функцию freeStudents, которая просматривает список студентов и освобождает все Student s и все StudentList элементы в нем. Затем вы вызываете эту функцию всякий раз, когда хотите избавиться от списка студентов.

Answer 3

Я думаю, у вас есть проблема в alloc.

Если вы хотите указатель, он должен быть

StudentList *p;
p = (StudentList*)malloc(sizeof(StudentList));

Answer 4

Вам действительно нужно определить, какие у вас типы. Не могли бы вы отредактировать свой пост, включив в него определения Student , StudentList и Students ?

Кроме того,

StudentList p;
p = (StudentList) malloc(sizeof(*p);

Если StudentList является typedef для указателя, я не уверен, как он компилируется.

У нас также есть строка:

StudentList p=students->students

С р определяется как Учащиеся . Тогда студенты должны быть typedef для указателя.

---

Кроме того, я думаю, что в конечном итоге ваша проблема заключается в том, что когда вы пытаетесь вставить учащегося в связанный список, вы теряете любой существующий список. Вам было бы интересно вставить в список 3 ученика, а затем попытаться распечатать список.

Answer 5

Вы должны освободить память, когда вам больше не нужно использовать выделенные вами объекты.

Что такое StudentList и Student? Мне кажется, что ваша структура данных построена неправильно. Почему ваш StudentList хранит указатель на другой StudentList? Вы, вероятно, должны создать StudentList, используя malloc и вернуть указатель на него, используя вашу функцию newStudents, и вместо того, чтобы возвращать новую структуру StudentList с копией старой в addStudent, просто измените структуру, которую вы создан изначально.

Так что это будет примерно так:

StudentList *newStudentList() {
    //malloc a new StudentList and return the pointer
}
void freeStudentList(StudentList *list) {
    //free the list pointer and all its child resources (i.e. the students in the list)
}
Student *newStudent(const char *name, etc) {
    //malloc a new Student and return the pointer
}
void addStudentToList(StudentList *list, Student *student) {
    //modify the StudentList which has been passed in by adding the student to it.
}

Вы можете объединить newStudent в addStudent, если вы не планируете использовать их отдельно, и вам могут понадобиться функции freeStudent и removeStudentFromList, если вы планируете делать эти вещи отдельно от freeStudentList. Посмотрите в Google примеры динамических структур данных в C ( здесь - первый результат в Google, но есть много других).

Answer 6

Я предполагаю, что StudentList определен как указатель на тип Student. В таком случае, эта строка в вашей функции alloc является проблемой. p=(StudentList)malloc(sizeof(*p))

Вы пытаетесь разыменовать указатель до того, как он был назначен, и это не хорошо.

Answer 7

Что это делает:

if (!(p=(StudentList)malloc(sizeof(*p)))){
    free(p);

Итак, если p не может быть выделено, освободите его?

Answer 8

Я никогда не использую sizeof с переменной, если использую malloc (). Вы должны использовать malloc(n * sizeof(StudentList)). С этим сказал ...

У вас есть серьезные проблемы. Прежде всего, вы не говорите нам, что конкретно определено для студентов и StudentList. Вы передаете 0 в некоторых случаях, когда я думаю, что вы имели в виду NULL - никогда не используйте 0, когда вы имеете в виду NULL. И если malloc () завершается ошибкой, то есть возвращается NULL, вы не вызываете free () для результата. Никогда, никогда не освобождай NULL.

Вы должны освобождать только блок памяти, который (а) был успешно выделен, и (б), когда он вам больше не нужен. Это, кажется, не входит в ваш код здесь.

Есть и другие проблемы (вы предполагаете, что учащиеся в addStudent не равны NULL, когда вы инициализируете p), но они не совсем решают ваш вопрос относительно использования free ().