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?

You need to call strtok 5 times - once for each field - before you can start copying the data to the struct. — 001
– 001, Commented Mar 3, 2021 at 20:12
Also, there's no need to call malloc and then strdup. You are only allocated room for one byte anyway - remove the malloc calls. — 001
– 001, Commented Mar 3, 2021 at 20:13
@JohnnyMopp I can't believe I wasted hours on something so trivial. Thank you! — Andrei0408
– Andrei0408, Commented Mar 3, 2021 at 20:33
@Calculuswhiz I've edited but I also changed the second loop. Everything that I did on name, county, grade, year should be in the first loop so that the words are first copied into fields. — Andrei0408
– Andrei0408, Commented Mar 3, 2021 at 20:36

Craig Estey · Accepted Answer · 2021-03-03 20:40:23Z

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;
}

Collectives™ on Stack Overflow

Segmentation Fault Dynamically allocated array of structures

1 Answer 1

Comments

Your Answer

Linked

Hot Network Questions

Collectives™ on Stack Overflow

1 Answer 1

Comments

Your Answer

Sign up or log in

Post as a guest

Linked

Related