I cannot read string after realloc

Question

I have this function, I realocate memory, but when i want to read string it doesn't work=>error Student is a Struct

void insertStudent(Student **myStudents, int *size)
{
    int newSize = *size + 1;
    *myStudents = (Student*)realloc(myStudents, newSize*sizeof(Student));
    printf("Enter your grade: ");
    scanf("%f", &(*myStudents)[*size - 1].nota);

    printf("Enter your first name: ");
    scanf("%s", &(*myStudents)[newSize-1].firstName);


    printf("Enter your second name: ");
    scanf("%s", &(*myStudents)[*size - 1].lastName);
    //Generate a new code
    /*int code = rand() % 1+1000;
    int ok = 0;
    while (ok == 0)
    {
        ok = 1;
        for (int i = 0; i < *size; i++)
            if ((*myStudents)[i].cod == code)
            {
                code = rand() % 1 + 1000;
                ok = 0;
            }
    }*/
    (*myStudents)[*size-1].cod = 7;
    printf("Your code is: %d. Do not forget it! ", 7);
}

You need to decide which of `*size - 1` and `newSize - 1` is correct and use that consistently. I think it is the second, but look carefully. — Jonathan Leffler, Jan 09 '18 at 08:34
Sidenote: **`realloc()` can fail.** In that case it returns `NULL`, *and the old pointer remains valid*. As you'd need to `free()` the **old** pointer, it's basically a memory leak waiting to happen to assign the return value of `realloc()` to the same pointer you passed as parameter. — DevSolar, Jan 09 '18 at 08:36
as others will (or have already) point out: Please don't cast the return of `mailloc`, `realloc` and `calloc` when writing plain old C. The cast is required in C++, but is considered bad practice in C as it can hide errors (especially pre C11) — Elias Van Ootegem, Jan 09 '18 at 10:38
@EliasVanOotegem nitpick: implicit function declaration is already wrong (requiring a diagnostic) in C99, making the "canonic" argument kind of weak. Nevertheless, I still think [casting to/from `void *` is bad practice](https://stackoverflow.com/a/45713482/2371524). — , Jan 09 '18 at 10:49
@MayankJain.: I guess you have resolved the problem you had. — user2736738, Jan 09 '18 at 11:54

score 4 · Answer 1 · 2018-01-09T08:41:18.163

void insertStudent(Student **myStudents, int *size)
{
    *myStudents = (Student*)realloc(myStudents, newSize*sizeof(Student));
                               //   ^ <- look here

This is a pointer to a pointer to your students. realloc() expects the pointer to the data originally allocated, so you most certainly have to pass *myStudents here!

Also change the code to use a temporary variable. realloc() may return NULL on error, in which case the original memory is still allocated and you have to free() it.

For calculating the size, it's better to use the expression syntax of sizeof (newSize * sizeof **myStudents) because this prevents errors when later changing the type.

For sizes, you should always use size_t, as this is guaranteed to hold any possible size of an object (int is not ...)

Further side note: converting to and from void * is implicit in C and it's arguably better style not to write this cast explicitly.

All in all, the code should be written like this

void insertStudent(Student **myStudents, size_t *size)
{
    size_t newSize = *size + 1;
    Student *newStudents = realloc(*myStudents, newSize * sizeof *newStudents);
    if (!newStudents)
    {
        free(*myStudents);
        *myStudents = 0;
        return; // check for this error in calling code
    }
    *myStudents = newStudents;
    *size = newSize;
    // [...]
}

user2736738 · Answer 2 · 2018-01-09T08:56:53.810

You are reallocting to myStudents. It is not your intention and it is not possible.

From standard 7.22.3.5

void *realloc(void *ptr, size_t size);

Otherwise, if ptr does not match a pointer earlier returned by a memory management function, or if the space has been deallocated by a call to the free or realloc function, the behavior is undefined. If memory for the new object cannot be allocated, the old object is not deallocated and its value is unchanged.

Earlier you had undefined behavior, you didn't pass the originally allocated memory's address. Rather you passed a local variable. You had undefined behavior.

Student* t = realloc(*myStudents, newSize*sizeof(Student))
if(t){
  *myStudents = t;
  (*size)++;
}
else {
   perror("realloc failed");
   free(*myStudents);
   exit(EXIT_FAILURE);
}

Also as you are increasing the memory you should do increase it if the call is successful. Then consistently access *size-1 throughout the code, it is easier to handle and go through.

The right way to do it shown above. In case realloc returns NULL you won't lose the reference to the already allocated memory. Also with this comes the advise to check the return value of realloc. Casting is redundant in this case - don't do it.

In the scanf you can simply write

scanf("%s",(*myStudents)[newSize-1].firstName);

otherwise you were passing char (*)[] it expects char*.

@MareșȘtefan.: Hope there is some memory allocated and it is not just pointer. — user2736738, Jan 09 '18 at 09:49

score 1 · Answer 3 · answered Jan 09 '18 at 08:25

1

realloc() needs pointer to the memory to be reallocated which is *myStudents and not myStudents.

Change

 *myStudents = (Student*)realloc(myStudents, newSize*sizeof(Student));

to

 *myStudents = (Student*)realloc(*myStudents, newSize*sizeof(Student));

answered Jan 09 '18 at 08:25

Gaurav Sehgal

7,422
2
18
34

I cannot read string after realloc

3 Answers3