Assigned value in array changes after being assigned

Question

In my code I need to build a function that's going to parse the content of a file and transform it into structures. The issue is that in that function in particular, on the first loop of the while, the resultats is going to have the right value in the array titres, but as soon as the new line is read (fgets(line, MAX_LINE_LENGTH, file)), and the value of line changes, the value inside resultats->titres changes too. I'm relatively new to C and can't figure out why as I manage to have my functions working elsewhere(see end of post). I have a feeling it's related to my limited understanding of pointers but I can't seem to figure out how to fix it !

If anyone could help me it would be greatly appreciated !

######## Code ########

structures: I didn't include it all but both structures have some getters and setters as well as a constructor and destructor

struct titre {
    char* tconst;
    char* primaryTitle;
    char* genres;
    char* titleType;
    int startYear;
};
typedef struct titre* t_titre;


struct resultats {
    int size;
    t_titre titres;
};
typedef struct resultats* t_resultats;

t_titre creer_titre(void) {
    t_titre titre;

    titre = (t_titre) malloc(sizeof(struct titre));
    memset(titre, 0, sizeof(*titre));

    if (titre) {
        titre->tconst = NULL;
        titre->primaryTitle = NULL;
        titre->genres = NULL;
        titre->titleType = NULL;
        titre->startYear = -1;
    }

    return titre;
}
 ...

void set_tconst(t_titre titre, char* tconst) {
    titre->tconst = tconst;
}
 ...

Not working implementation:

#include "imdb.h"
#define MAX_LINE_LENGTH 128

void parse_line(t_titre titre, char* line);
void filter_results(t_resultats results, t_critere filter);

t_resultats search_file(t_critere criteres, char* file_name) {
    FILE* file = fopen(file_name, "r");
    t_resultats resultats = creer_resultats();
    t_titre titre = creer_titre();

    if (file == NULL) {
        printf("Couldn't open file %s\n", file_name);
        exit(EXIT_FAILURE);
    }

    char line[MAX_LINE_LENGTH];
    fgets(line, MAX_LINE_LENGTH, file); //Skip header line 
    while (fgets(line, MAX_LINE_LENGTH, file)) {
        parse_line(titre, line);
        add_titre(resultats, titre);
    }

    free(titre);
    fclose(file);
    return NULL;
}

void parse_line(t_titre titre, char* line) {
    line[strcspn(line, "\n")] = 0; //Remove trailling \n

    set_tconst(titre, strsep(&line, "\t"));
    set_titleType(titre, strsep(&line, "\t"));
    set_primaryTitle(titre, strsep(&line, "\t"));
    strsep(&line, "\t"); //Skip originalTitle
    strsep(&line, "\t"); //Skip isAdult
    set_startYear(titre, strtol(strsep(&line, "\t"), NULL, 10));
    strsep(&line, "\t"); //Skip endYear
    strsep(&line, "\t"); //Skip runtimeMinutes
    set_genres(titre, line); //Use remaining of line as it is the last value
}

void filter_results(t_resultats results, t_critere filter) {

}

Working implementation:

    t_resultats resultats = creer_resultats();
    t_titre t = creer_titre();

    set_tconst(t, "tt0000001");
    set_titleType(t, "short");
    set_primaryTitle(t, "It");
    set_startYear(t, 2015);
    set_genres(t, "Horror,Short");
    add_titre(resultats, t);

    set_tconst(t, "tt0000002");
    set_titleType(t, "short");
    set_primaryTitle(t, "ET");
    set_startYear(t, 2015);
    set_genres(t, "Horror,Short");
    add_titre(resultats, t);

    set_tconst(t, "tt0000003");
    set_titleType(t, "short");
    set_primaryTitle(t, "Interstellar");
    set_startYear(t, 2015);
    set_genres(t, "Sci-Fi,Short");
    add_titre(resultats, t);

There is a long time I don't look at C, but it seems that you are passing `titre` by value and not as a pointer. To pass as pointer you should `(t_titre *titre)` and when calling it you should call like `func( &titre)`. Take a look at this [link](https://www.tutorialspoint.com/cprogramming/c_passing_pointers_to_functions.htm) here. — Joao Soares, Jan 27 '22 at 04:13
@JoaoSoares It is actually a pointer as I use the typedef do define a type equal to a pointer to my structure ! (I hate it but it's a requirement of the exercise). Thanks tho =) — Quentin Levasseur, Jan 27 '22 at 04:17
@QuentinLevasseur, can you show the body of `set_tconst()` for example — kuro, Jan 27 '22 at 05:02
@kuro I just Edited the post to add an example of getter and constructor — Quentin Levasseur, Jan 27 '22 at 05:15
In `set_tconst()` you are just assigning the pointer in `titre->tconst = tconst`. So, for `set_tconst()` you are now pointing to the address of say `line[0]`. But in the next iteration of while loop you overwrite the previous content of `line`. So `titre->tconst` will also point to new value. To fix this, you have to allocate memory for `titre->tconst` and then copy the content `tconst` parameter in that. — kuro, Jan 27 '22 at 05:19
Your second code works basically because the pointer you are passing to `set_tconst()` are different. — kuro, Jan 27 '22 at 05:20
@kuro So something likle this ? ```void set_tconst(t_titre titre, char* tconst) { titre->tconst = (char*) malloc(sizeof(tconst)); titre->tconst = tconst; }``` — Quentin Levasseur, Jan 27 '22 at 05:27
@kuro I fugured they were different but in that case why would this not work ? ``` while (fgets(line, MAX_LINE_LENGTH, file)) { t_titre titre = creer_titre(); parse_line(titre, line); add_titre(resultats, titre); free(titre); }``` — Quentin Levasseur, Jan 27 '22 at 05:31
not quite. you need `malloc` but the `sizeof(tconst)` won't do, because it represent sizeof a char* not the length of the string. Also the next assignment won't work either. You need something like `memcpy()` or `strcpy()` — kuro, Jan 27 '22 at 05:32
@kuro here is **add_titre**: ```void add_titre(t_resultats resultats, t_titre titre) { resultats->size++; resultats->titres = realloc(resultats->titres, resultats->size * sizeof *resultats->titres); resultats->titres[resultats->size - 1] = *titre; }``` — Quentin Levasseur, Jan 27 '22 at 05:36
As for typedeffing pointers, you will want to review: [Is it a good idea to **typedef** pointers?](http://stackoverflow.com/questions/750178/is-it-a-good-idea-to-typedef-pointers). — David C. Rankin, Jan 27 '22 at 05:37
@DavidC.Rankin i don't have a choice with that sadly ! It's a requirement of the teacher — Quentin Levasseur, Jan 27 '22 at 05:39
It happens. The crux of it is, typedeffed pointers can cause confusion as to the type, especially 1000 lines down in your code when a type pops up for the first time. That said, they have their place. The kernel code makes extensive use of them for list-pointers, etc. The general consensus is against typedeffing pointers, but with the caveat that there are places where there are accepted cases for using them. — David C. Rankin, Jan 27 '22 at 06:39

score 0 · Accepted Answer · answered Jan 27 '22 at 05:53

The answer was simple, I needed to change the setter methods in order to allocate memory then copy the value :

This :

void set_tconst(t_titre titre, char* tconst) {
    titre->tconst = (char*) malloc(sizeof(tconst));
    strcpy(titre->tconst, tconst);
}

Instead of this:

void set_tconst(t_titre titre, char* tconst) {
    titre->tconst = tconst;
}

Thanks everyone !

Assigned value in array changes after being assigned

1 Answers1