How to correctly malloc a struct in C

Question

Here is my full code, it looks like to work, but it's not working very well. I would accept any code, that is working like this.

Firstly, the code works, but when I want to add the third name to the struct, it crashes.

Is there any other way to do this?

I need struct, because in the future, I want to add some other params, like age, average, gender, etc.

Please, help me out.

//The student table
typedef struct students {
    char name[50];
} students;

//Global params
int scount = 0;
students *s;

//Basic functions
void addNewStudent();

int main()
{
    int loop = 1;
    char in;
    int ch;
    printf("Willkommen.\n Wahlen Sie bitte von die folgenden Optionen:\n");
    while (loop)
    {
        printf("\t[1] Neue Student eingeben\n");
        printf("\t[9] Programm beenden\n");

        scanf(" %c", &in);
        while ((ch = getchar()) != '\n');
        switch (in)
        {
        case '1':
            addNewStudent();
            break;
        case '9':
            loop = 0;
            break;
        default: printf("------\nOption nicht gefunden.\n------\n");
            break;
        }
    }
    free(s);
    return 0;
}

void addNewStudent()
{
    int index = 0;
    if (scount == 0)
    {
        s = (students*)malloc(sizeof(students));
    }
    else
    {
        realloc(s, sizeof(students) * scount);
    }

    printf("Geben Sie Bitte die Name:\n");
    fgets(s[scount].name, sizeof(s[scount].name), stdin);

    while (s[scount].name[index] != '\n')
    {
        index++;
    }
    s[scount].name[index] = '\0';
    scount++;
}

I'm using Visual Studio.

Thanks for help!

Answer 1

students *mynew= realloc(s, sizeof(students)* (scount+1));
if( mynew != NULL )
    s=mynew;

Otehrwise you are having a memory leak. You didn't use the return value of realloc.

Don't cast the return type of malloc.

As per standard §7.22.2.35

void *realloc(void *ptr, size_t size)

The realloc function deallocates the old object pointed to by ptr and returns a pointer to a new object that has the size specified by size.

It is good not to use the same pointer variable on which you are calling malloc because in case it fails you will lose reference to the old one too (unless it is stored by other means).

Also you didn't check the return value of malloc.

s = malloc(sizeof(students));
if( s == NULL ){
   frpntf(stderr,"%s","Memory allocation failed");
   exit(1);
}

Also you should check the return value of fgets().

if( fgets(s[scount].name, sizeof(s[scount].name), stdin) == NULL){
     fprintf(stderr,"%s","Error in input");
     exit(1);
}

---

Also trying to compile your code it showed this

warning: ignoring return value of ‘realloc’, declared with attribute warn_unused_result [-Wunused-result]
         realloc(s, sizeof(students) * scount);
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

When compiling try not to ignore any warning messages. It showed the problem you had.

Important point: (why scount+1 in realloc?)

When reallocating the general idea is increase the number of students. And for that you need to have extra memory allocated for an student. That's why the scount+1 in the code.(realloc).

---

Some other points:

while (s[scount].name[index] != '\n')
{
    index++;
}
s[scount].name[index] = '\0';

You can do it like this also

size_t len = strlen(s[scount].name);
if(len){ 
   s[scount].name[len-1]='\0'; 
}

To understand why from standard §7.21.7.2

char *fgets(char * restrict s, int n,FILE * restrict stream)

The fgets function reads at most one less than the number of characters specified by n from the stream pointed to by stream into the array pointed to by s. No additional characters are read after a new-line character (which is retained) or after end-of-file. A null character is written immediately after the last character read into the array.

\0 character was there already in the inputted string. You can get the length of it but you know that the one before the \0 is the \n character ¹ that you entered by pressing the Enter key. We are overwriting it with the \0.

1. This is the usual case but not the only one. There are two cases where this might not be the right way to look at the thing.

• The input line has n-1 or more characters before the '\n'. The the one before \0 will not be the \n rather it will be some character inputted by the user.

• The last line is a stream which may not have a '\n'. (stdin closed). In that case also the input doesn't contain the \n.

So in these cases the idea of removing \n would fail._{Discussed in comment. (chux)}

---

A better and safe solution than overwriting this way:

s[scount].name[strcspn(s[scount].name, "\n")] = '\0';

The explanation from the link is that if a \0 is given as input then we will basically write to s[scount].name[SIZE_MAX] which is not desired.

---

From the standard §7.24.5.3

size_t strcspn(const char *s1, const char *s2)

The strcspn function computes the length of the maximum initial segment of the string pointed to by s1 which consists entirely of characters not from the string pointed to by s2.

Answer 2

You are not allocating it back! Take a look at how realloc works. You need to assign the pointer back after making the re-allocation like this.

if (scount == 0)
{
    s = (students*)malloc(sizeof(students));
}
else
{
    students *temp = realloc(s, sizeof(students) * (scount+1));
    if(temp == NULL){
        free(s);
    }
    else{
        s = temp;
    }
}

By Definition, realloc returns a void pointer but you aren't collecting it.

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

realloc returns a NULL if there's not enough space. So you can re-assign it when you are sure that it is not NULL

Just make a small change above and your code works like a charm!

Cheers!

Answer 3

How to correctly malloc a struct in C ?

p = malloc(sizeof *p);
if (p == NULL) Handle_OutOfMemory();

How to correctly re-allocate a struct in C ?

void *t = realloc(p, sizeof *p * number_of_elements);
if (t == NULL && number_of_elements > 0) {
  Handle_OutOfMemory();
} else {
  p = t;
}

p points to some struct. Notice no coding of that type in above.

---

OP' primary problem is not using the return value of realloc() and allocating 1-too-small

// realloc(s, sizeof(students) * scount);
s = realloc(s, sizeof *s * (scount+1));  // or use above code with check for out-of-memory.

Answer 4

realloc returns a new pointer that you need to keep:

  students* snew = realloc(s, sizeof(students) * (scount + 1));
  if (!snew) {
     free(s); // If there is not enough memory, the old memory block is not freed
     // handle out of memory
  } else { 
     s = snew;
  }