Freeing an array of structures which has char pointers

Question

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
typedef struct
{
int id;
char *name;
char *lastname;
} student_type;
typedef struct
{
student_type list[15];
} student_list_type;
void get_students(FILE *input,char *filename,student_list_type *student_list);
int main()
{
printf("Hello world!\n");
student_list_type std_list;
student_list_type *std_list_p=&std_list;
 FILE *input;
get_students(input,"students.txt",std_list_p);
return 0;
}
void get_students(FILE *input,char *filename,student_list_type *student_list)
{

int i=0;
int j=0;
input=fopen(filename,"r");
printf("filename is %s",filename);
while(fscanf(input,"%d",&student_list->list[i].id)==1)
{
    student_list->list[i].name=(char *) malloc(15);
    student_list->list[i].lastname=(char *) malloc(15);
    fscanf(input,"%s",student_list->list[i].name);
    fscanf(input,"%s",student_list->list[i].lastname);
    i++;
}

for(j=0; j<i+1; j++)
{
    free(student_list->list[i].name);
    free(student_list->list[i].lastname);
}
free(student_list->list);
fclose(input);
}

I think i am failing at freeing part of this code.I have learned that i should free elemenets first then whole array but i possibly learned wrong way.Anyways this code doesn't give error messages but simply crashes sometimes after getting all student list from txt.

score 0 · Answer 1 · answered Mar 20 '13 at 15:40

First of all you don't check if your fopen succeeds, if it fails fscanf still tries to open the variable input but in this case that would be NULL.

And why do you declare "File * input;" in main? If you pass it in the next moment to your function "get_student". Why not declare it directly in the function?

This code is more fail safe

void get_students(char *filename,student_list_type *student_list)
{
    int i=0;
    int j=0;

    FILE * input;

    input=fopen(filename,"r");
    printf("filename is %s",filename);   

    if((input = fopen("students.txt","r")) == NULL)
    {
        fprintf(stderr, "\nFile could not be open\n");
    }
    else
    {
        while(fscanf(input,"%d",&student_list->list[i].id)==1)
        {
            student_list->list[i].name=(char *) malloc(15);
            student_list->list[i].lastname=(char *) malloc(15);
            fscanf(input,"%s",student_list->list[i].name);
            fscanf(input,"%s",student_list->list[i].lastname);
            i++;
           }

            for(j=0; j<i+1; j++)
            {
                free(student_list->list[i].name);
                free(student_list->list[i].lastname);
            }

            fclose(input);
        }
}

And at the end you don't need to free this

free(student_list->list);

Because student_list was not allocated on the heap.

To my defense,i declared file input in main since i remember that lazy-amateur usage worked for my last homework.You know what they say "If it works,don't fix it".Though i would be careful to gain more professional approach . For freeing part i allocated (at least i tried) student_list->list elemenets by ; student_list->list[i].name=(char *) malloc(15); student_list->list[i].lastname=(char *) malloc(15); you tell me they are not allocated in heap already.Do you tell it because i allocated in function definition since they will be gone anyways or i failed allocating them at all? — Mehmet Çağrı Köse, Mar 20 '13 at 17:18
They will be gone anyways, you didn' fail at this point. You allocated them ealier on the stack. Take a look at this to understand it better. http://stackoverflow.com/questions/79923/what-and-where-are-the-stack-and-heap — akristmann, Mar 21 '13 at 07:36

Freeing an array of structures which has char pointers

1 Answers1