Is this reallocation method correct

Question

My code works but my question is if this dynamic allocation is correct. It's works well and all is ok but I'm not so sure it is correct.

        StudentDynamic* pStudents = NULL;
        char auxfirstName[255], auxlastName[255];
        float auxGrade;
        FILE* pFile = fopen("InitialDB.txt", "r");
        if (pFile == NULL)
        {
            printf("Could not open file or is empty! Exiting...");
            exit(2);
        }
        int i = 0;
        pStudents = (StudentDynamic*)malloc(sizeof(StudentDynamic) * 1);
        while (!feof(pFile))
        {
            fscanf(pFile, "%s", auxfirstName);
            pStudents[i].firstName = (char*)malloc(strlen(auxfirstName) + 1);
            strcpy(pStudents[i].firstName, auxfirstName);

            fscanf(pFile, "%s", auxlastName);
            pStudents[i].lastName = (char*)malloc(strlen(auxlastName) + 1);
            strcpy(pStudents[i].lastName, auxlastName);

            fscanf(pFile, "%f", &auxGrade);
            pStudents[i].grade = auxGrade;

            i++;
            pStudents = (StudentDynamic*)realloc(pStudents, sizeof(StudentDynamic) * (i + 1));
        }
        nStudents = i;
        fclose(pFile);

[Please see this discussion on why not to cast the return value of malloc() and family in C..](https://stackoverflow.com/q/605845/2173917) — Sourav Ghosh, Dec 08 '17 at 15:38
[Please see Why is “while ( !feof (file) )” always wrong?](https://stackoverflow.com/q/5431941/2173917) — Sourav Ghosh, Dec 08 '17 at 15:39
if it's correct maybe corereview would be a better place to get comments. — Jean-François Fabre, Dec 08 '17 at 15:40
I'm voting to close this question as off-topic because it belongs to http://codereview.stackexchange.com — Jean-François Fabre, Dec 08 '17 at 15:40
Ok, i will read but my misunderstanding is if it's correct the part with declaring char auxfirstName[255], auxlastName[255]; and then do the dynamic allocation. — Bogdy, Dec 08 '17 at 15:41
I'm still wondering if casting mallocs is really as bad as some of you say. I know "old-school" C programmers that prefer to keep the cast. I am really a newby and this confuses me a lot. — PySerial Killer, Dec 08 '17 at 15:43

score 1 · Answer 1 · answered Dec 08 '17 at 15:40

1

temp_pStudents  = realloc(pStudents , sizeof(StudentDynamic) * (i + 1));
if (!temp_pStudents)
    // error
pStudents  = temp_pStudents ;

Ideally this would be something like this. Otherwise in case of error you have a memory leak. Also this saves you from derefrencing the null pointer.

Again other things include

casting malloc which is not necessary.
And using constructs with while(!feof(file)). Don't use it. Check the discussion posted in comment.

answered Dec 08 '17 at 15:40

user2736738

30,591
5
42
56

temp_pStudents how should be declared? – Bogdy Dec 08 '17 at 15:46
@Bogdy.: `StudentDynamic *temp_pStudents `. – user2736738 Dec 08 '17 at 15:47
Now I get crash.. Could you check if you want, please? https://pastebin.com/kFGLRN54 – Bogdy Dec 08 '17 at 15:48
@Bogdy.: Your code has undefined behavior. `fflush(stdin)` etc...Make changes to it or ask proper question or even better get a book. – user2736738 Dec 08 '17 at 15:56

Is this reallocation method correct

1 Answers1