Proper way to fill nested struct in C from file

Question

I read data from a file that I want to fit into a structure. This structure contains an array of variable size. So I am using realloc to fill it on the fly. Unfortunately my program fails. Valgrind reports a pb with realloc:

==3170== Invalid write of size 8
==3170==    at 0x1093E6: charge_Un_BV (charge_un_bv.c:56)
==3170==    by 0x109193: main (charge_un_bv.c:87)
==3170==  Address 0x4e507a8 is 8 bytes before a block of size 8 alloc'd
==3170==    at 0x4A3F2CC: realloc (in /home/linuxbrew/.linuxbrew/Cellar/valgrind/3.17.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==3170==    by 0x1093CD: charge_Un_BV (charge_un_bv.c:53)
==3170==    by 0x109193: main (charge_un_bv.c:87)

Here my program:

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

typedef struct st_BV {
    char nom_BV[40];
    int  id_bv;
    long  tabIndicePointsLength;
    long*  tabIndicePoints;
} BV;

BV* charge_Un_BV(char*); 
 
BV* charge_Un_BV(char* nomFileBV)
{
    BV* oneBV;
    oneBV = (BV*)malloc(sizeof(BV));
    if (oneBV == NULL) { printf("Erreur allocation mémoire\n"); exit(-6); }

    FILE* pt_fichier_BV = fopen(nomFileBV,"r");
    char ligne[100];
    long indice;
    float lon,lat;
    long count_ligne = 0;

    printf("start\n");  

    if (pt_fichier_BV == NULL)
    {
        printf("unable to open %s\n", nomFileBV);
        exit(-1);
    }
    else
    {
        printf("File opened\n");
        char* resultat = fgets(ligne,100,pt_fichier_BV);
        if (resultat == NULL) {printf("Error reading file\n"); exit(-2);}
        printf("First ligne skip: %s",ligne);
        
        
        oneBV->tabIndicePoints = (long*)malloc(sizeof(long));
        if (oneBV->tabIndicePoints == NULL) { printf("Error with alloc\n"); exit(-3); }

        int cr;
        
        do{
            cr = fscanf (pt_fichier_BV,"%ld %f %f",&indice, &lon, &lat);

            // reallocation according to the number of points read in the file
// BELOW LINE 53 :
            oneBV->tabIndicePoints = (long*)realloc(oneBV->tabIndicePoints,(count_ligne+1) * sizeof(long));
            if (oneBV->tabIndicePoints == NULL) { printf("Error realloc\n"); exit(-4); }
            
// BELOW LINE 56 :
            oneBV->tabIndicePoints[count_ligne-1] = indice;
            count_ligne++;
        }while(cr!=EOF);
        //total number of points
         oneBV->tabIndicePointsLength = count_ligne-1;
  //TEST
        printf("First indice (index): %ld\n",oneBV->tabIndicePoints[0]);
        printf("Fourth  indice (index): %ld\n",oneBV->tabIndicePoints[3]);
        printf("Total points: %ld\n",  oneBV->tabIndicePointsLength);

    }
    //close file
    int cr = fclose(pt_fichier_BV);
    if (cr != 0)
    {
        printf("Erro closing file\n");
        exit(-5);
    }   
    return oneBV;
}


// Programme TEST
int main()
{
    BV* lebv;
// BELOW LINE 87 :
    lebv = charge_Un_BV("../data/data.txt");
    
    free (lebv);

    return 0;
}

So I am wondering what is the correct way to fill a dynamic array with a structure. regards

EDIT AFTER FIRST SOLUTION

I add this problem to my first post. valgrind report still gives me error about memory leak. I guess, this comes from the first alloc just before the realloc or from the last realloc. So I have a block of memory not freed. This does not prevent the program from working but can we improve it? And I added "free (lebv);"

==2858== 
début
Fichier ouvert
Première ligne: ind0 lon lat
Premier indice: 584119
Quatrième  indice: 584120
Nombre de points: 871
Quatrième  indice (2): 584120
==2858== 
==2858== HEAP SUMMARY:
==2858==     in use at exit: 6,976 bytes in 1 blocks
==2858==   total heap usage: 877 allocs, 876 frees, 3,050,688 bytes allocated
==2858== 
==2858== 6,976 bytes in 1 blocks are definitely lost in loss record 1 of 1
==2858==    at 0x4A3F2CC: realloc (in /home/linuxbrew/.linuxbrew/Cellar/valgrind/3.17.0/libexec/valgrind/vgpreload_memcheck-amd64-linux.so)
==2858==    by 0x1093ED: charge_Un_BV (charge_un_bv.c:53)
==2858==    by 0x1091B0: main (charge_un_bv.c:87)
==2858== 
==2858== LEAK SUMMARY:
==2858==    definitely lost: 6,976 bytes in 1 blocks
==2858==    indirectly lost: 0 bytes in 0 blocks
==2858==      possibly lost: 0 bytes in 0 blocks
==2858==    still reachable: 0 bytes in 0 blocks
==2858==         suppressed: 0 bytes in 0 blocks
==2858== 
==2858== ERROR SUMMARY: 1 errors from 1 contexts (suppressed: 0 from 0)

In C, there is no need to cast the return of `malloc`, it is unnecessary. See: [Do I cast the result of malloc?](http://stackoverflow.com/q/605845/995714) Normally the `'*'` goes with the *pointer* and not the *type* at declaration. Why? `char* a, b, c;` does NOT declare three pointers to `char`, it declares 1 pointer and 2 character variables. `char *a, b, c;` makes that clear. — David C. Rankin, Jun 08 '21 at 20:20
Thank you for these tips, I will apply them from now on. However, I find that the char* a; notation is more explicit if we are aware of your remarks. — user2030243, Jun 09 '21 at 07:33
As long as you are clear on the effect, that is all that matters. Either way is perfectly okay. — David C. Rankin, Jun 09 '21 at 07:54

score 2 · Accepted Answer · answered Jun 08 '21 at 19:41

2

You initialize count_ligne to 0

    long count_ligne = 0;

and later subtract 1 from it before incrementing it and use it as an index (accessing index -1)

            oneBV->tabIndicePoints[count_ligne-1] = indice;
            count_ligne++;

You probably want to either:

not subtract 1, or
increment before using it

answered Jun 08 '21 at 19:41

idz

12,825
1
29
40

Thank you this is indeed the problem, I wonder how I could not see it. However I have another concern valgrind gives me a memory leak which does not prevent the program from working well. I will edit my post about that. – user2030243 Jun 09 '21 at 07:36
Finally I found the leak. You have to do: `free (lebv.tabIndicePoints);` – user2030243 Jun 09 '21 at 11:03
Glad you found the problem. If you have a follow on question after your initial question is answered, you should open another question. You'll get faster response and make it easier for other users to find solutions when they have similar problems. – idz Jun 09 '21 at 18:57

Proper way to fill nested struct in C from file

1 Answers1