Segmentation Fault Dynamically allocated array of structures

Question

I want to parse a .csv file of the form: grade(float), year(int), name(string), county(string), number(int) and I don't see my mistake:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX 300

typedef struct{
    char *name, *county;
    float grade;
    unsigned long int year, number;
}info;

int main(int argc, char **argv)
{
    FILE *fin = fopen("lab3.txt", "r");
    if(!fin)
    {
        fprintf(stderr, "ERROR OPENING THE FILE.\n");
        exit(1);
    }
    char buf[MAX];
    while(fgets(buf, MAX, fin))
    {
        buf[strcspn(buf,"\n")] = '\0'; // remove the trailing newline
        char *fields[6], *word = strtok(buf, ",");
        int i = 0;
        while(word)
        {
            info *array = (info *)malloc(sizeof(info));
            fields[i] = word;
            array->name = strdup(fields[2]);
            array->county = strdup(fields[3]);
            array->grade = atof(fields[0]);
            array->year = atoi(fields[1]);
            array->number = atoi(fields[4]);
            printf("Name : %s | County: %s | Year : %ld | Grade : %f | Number: %ld",array->name, array->county, array->year, array->grade, array->number);
            //printf("Word : %s\n", fields[i]);
            word = strtok(NULL,",");
            i++;
            free(array->county);
            free(array->name);
            free(array);

        }
    }
    fclose(fin);
    return 0;
}

There are exactly 5 fields on each line, so I wanted to break each line into words andI also used gdb to check what's wrong and the problem seems to be here array->name = strdup(fields[2]);. I've tried numerous things so far, I've allocated memory for the array and the name and county, I'm freeing the memory so, what's the mistake?

Answer 1

There are a number of issues.

You don't really need [nor want] either fields or word. You can just use realloc to increase the size of the array and operate on the current pointer directly.

Doing malloc(sizeof(char)), as you have it, just leaks memory because of the strdup after it.

You do not want to do free at the bottom of the loop--it just kills what you did strdup for.

You are not expanding the size of array on each iteration [using realloc], so your array size stays at one. And, you blow away the previous value on each iteration, so, again, you're leaking memory.

Here's a refactored version:

#include <stdio.h>
#include <stdlib.h>
#include <string.h>

#define MAX 300

typedef struct {
    char *name;
    char *county;
    float grade;
    unsigned long int year;
    unsigned long int number;
} info;

#define TOK \
    ({ \
        cp = strtok(bp,","); \
        bp = NULL; \
        cp; \
    })

#define SAVE_S(_sym) \
    arrcur->_sym = strdup(TOK)
#define SAVE_F(_sym) \
    arrcur->_sym = atof(TOK)
#define SAVE_I(_sym) \
    arrcur->_sym = atol(TOK)

int
main(int argc, char **argv)
{
    FILE *fin = fopen("lab3.txt", "r");

    if (!fin) {
        fprintf(stderr, "ERROR OPENING THE FILE.\n");
        exit(1);
    }

    char buf[MAX];
    info *arrbase = NULL;
    int arrcnt = 0;
    int arrmax = 0;
    char *bp;
    char *cp;

    while (fgets(buf, MAX, fin)) {
        // remove the trailing newline
        buf[strcspn(buf, "\n")] = '\0';

        // enlarge array -- use of arrmax limits number of realloc calls
        if (arrcnt >= arrmax) {
            arrmax += 100;
            arrbase = realloc(arrbase,sizeof(*arrbase) * arrmax);
        }

        // point to current array entry
        info *arrcur = &arrbase[arrcnt];

        bp = buf;

        SAVE_S(name);
        SAVE_S(county);
        SAVE_F(grade);
        SAVE_I(year);
        SAVE_I(number);

        printf("Name : %s | County: %s | Year : %ld | Grade : %f | Number: %ld",
            arrcur->name, arrcur->county, arrcur->year, arrcur->grade,
            arrcur->number);

        ++arrcnt;
    }

    fclose(fin);

    // trim to size used
    arrbase = realloc(arrbase,sizeof(*arrbase) * arrcnt);

    return 0;
}