3

When I run the program and give values to the id, name, surname it gives them all the value of the last student. For instance if the last students name is Anna then all the other names of the array are Anna. With the grades it works well! I tried and without the 'constructor' function and happenden the same thing.

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


struct Student{     /*struct for student info*/
    char *id;
    char *name;
    char *surname;
    int grade;
};


struct Student* Student_new(char* id, char* name, char* surname, int grade); 
/*fuction: Student 'constructor'*/


int main(int argc, char *argv[]) {

int no; /*number of students*/
printf("Welcome! \n\nNumber of students: ");
scanf("%d", &no);

struct Student *studentArray[no]; /*arary of struct students*/

int i; /*counter*/
for(i=0; i<no; i++){

    char id[10], name[10], surname[10];
    int grade;

    printf("\n\nStudent(%d)\n", i+1);
    printf("id: ");
    scanf("%s", id);
    printf("name: ");
    scanf("%s", name);
    printf("surname: ");
    scanf("%s", surname);
    printf("grade: ");
    scanf("%d", &grade);


    studentArray[i] = Student_new(id, name, surname, grade); /*using Student 
    'constructor' to initialize the array*/
}



for(i=0; i<no; i++){
    printf("%s  %s %s %d \n", studentArray[i]->id, studentArray[i]-
    >name, studentArray[i]->surname, studentArray[i]->grade);
}

    return 0;
}


struct Student* Student_new(char* id, char* name, char* surname, int grade) 
{ 

      struct Student* st = malloc(sizeof(struct Student));
      st->id = id;
      st->name = name;
      st->surname = surname;
      st->grade = grade;
      return st;
}

PLEASE HELP!!

Spiros Kaloudis
  • 31
  • 2

3 Answers3

5

The issue is that the loop variables go out of scope after each iteration, and you're left with dangling pointers in the Student instances. What you're seeing is the result of undefined behavior.

What's probably happening is that the same char array is being passed into every student instance. Then you modify the same char array, overwriting the previous values.

You'll need to make copies of the strings. Remember to create a function like Student_free where you free the dynamically allocated copies.

struct Student* Student_new(char* id, char* name, char* surname, int grade) 
{ 

    struct Student* st = malloc(sizeof(struct Student));
    st->id = strndup(id, 10);
    st->name = strndup(name, 10);
    st->surname = strndup(surname, 10);
    st->grade = grade;
    return st;
}
cbr
  • 12,563
  • 3
  • 38
  • 63
  • If he is creaating name, surname etc each time in the loop, it doesen't seem he is using the same char arrays, or are they actually same. Please explain – Varun Garg May 26 '17 at 14:17
  • @VarunGarg The arrays go out of scope after the iteration, what happens to the values afterwards is undefined behavior (or unspecified, or implementation-defined, I don't know what the standard says). What is most likely happening is that the compiler allocates 10 bytes for each array on the stack of the `main` function, and every loop iteration manipulates these arrays. Since he prints the values in the same function, the values are still in memory since nothing has replaced them. But we just got lucky in this case. – cbr May 26 '17 at 14:23
  • You might be interested in these two questions: [*Is a variable declared in a loop body preserved during iterations?*](https://stackoverflow.com/q/11650905/996081) and [*Undefined, unspecified and implementation-defined behavior*](https://stackoverflow.com/q/2397984/996081) – cbr May 26 '17 at 14:26
  • 1
    thanks for explaination, Now I think that https://stackoverflow.com/a/13861140/3276830 also explains ops problem – Varun Garg May 26 '17 at 14:45
2

You should reserve memory for the string attributes. As a hint, use a struct similar to:

#define MAX_LEN 32

struct Student{     /*struct for student info*/
    char id[MAX_LEN];
    char name[MAX_LEN];
    char surname[MAX_LEN];
    int grade;
};

Define MAX_LEN to something that is reasonable for you, and check that the entered values aren't any longer. Also make sure to strcpy the input values to the struct variables.

nucleon
  • 1,128
  • 1
  • 6
  • 19
2

The student structure only holds pointers to char arrays. Now the actual space in memory for the strings is allocated in your for cycle an its lifetime is limited by the scope of the for cycle, accessing it afterwards is undefined behaviour. In your case the space where the strings are has not been reused and overridden yet, so coincidentally you are able to get the last value stored (cases like this can also raise segmentation fault).

You should have a look on some basic tutorial about pointers and c memory model. Allocating the arrays in the struct is a good solution (nucleon's answer). Also the scanf function can overflow you should limit the number of retrieved characters to match the size of allocated array.

Martin Tomaschek
  • 23
  • 5