1

I am trying to create 2D array of structures and print the value. How "Segmentaion fault (core dumped)" message". 

#include <iostream>
#include <string>
using namespace std;

struct student{
    string name;
    int age;
    float marks;
};
student* initiateStudent(string name, int age, float marks){
    student *studFun;
    studFun->name = name;
    studFun->age = age;
    studFun->marks =  marks;
    return studFun;  

}
int main() {
    int totalStudents = 1;
    string name;
    int age;
    float marks;
    cin >> totalStudents;
    student** stud = new student*[totalStudents];
    for(int i=0;i<totalStudents;i++){
        stud[i] = new student[1];
        cin >> name >> age >> marks;
        stud[i] = initiateStudent(name,age,marks);
    }

    delete [] stud;
    return 0;
}

I am compiling it using Netbeans for C++. Can anyone tell me whats wrong with this code ?

Spandyie
  • 914
  • 2
  • 11
  • 23
  • `initiateStudent` has an uninitialized variable `studFun` which you are then indirecting and writing to. That tends to crash. Find a compiler that gives decent warnings and turn them on at the maximum level. – Zalman Stern Dec 13 '17 at 06:20
  • There is an extra closing bracket at the end of initiateStudent function. – Aqeel Raza Dec 13 '17 at 06:23
  • @ZalmanStern How do you suggest I should modify my code? – Spandyie Dec 13 '17 at 06:36
  • @Spandy Why not simply use `std::vector>` and avoid these headaches? – PaulMcKenzie Dec 13 '17 at 06:49
  • I have two pieces of advice. The first is given above: get the compiler to show you the trivial errors.Second, get a clear picture in your mind of what you are trying to do. In particular you mention a 2D array, but as this stands, it is just a really inefficient way to store a single column. If you need sparse 2D representation, you might want to use two pointers like it is, or if the same student appears at multiple places, you might want to use a single 2D array of pointers. Otherwise allocating one block to hold all the structs and not calling new per struct is best. – Zalman Stern Dec 13 '17 at 07:01
  • @PaulMcKenzie But vectors wont let me store string, int and float in same std::vector>? – Spandyie Dec 13 '17 at 07:03
  • 1
    @Spandy: [Huh?](https://www.ideone.com/AvvQco). All this boils down to is a single array of `student`. In addition, that `initiateStudent` function need not exist. Just create a constructor for the `student` type. – PaulMcKenzie Dec 13 '17 at 07:05

1 Answers1

1

This should work

#include <iostream>
#include <string>
using namespace std;

struct student{
   string name;
   int age;
   float marks;
};
student* initiateStudent(string name, int age, float marks){
   student *studFun = new student();
   studFun->name = name;
   studFun->age = age;
   studFun->marks =  marks;
   return studFun;
}
int main() {
   int totalStudents = 1;
   string name;
   int age;
   float marks;
   cin >> totalStudents;
   student** stud = new student*[totalStudents];
   for(int i=0;i<totalStudents;i++){
       stud[i] = new student[1];
       cin >> name;
       cin >> age;
       cin >> marks;
       stud[i] = initiateStudent(name,age,marks);
  }

  delete [] stud;
  return 0;
}
Aqeel Raza
  • 1,729
  • 4
  • 15
  • 24
  • Just wondering how is student *studFun = new student(); different from student *studFun;? – Spandyie Dec 13 '17 at 06:42
  • 1
    student *studFun 1 "test", being an uninitialized pointer to a "studFun" structure, can be directing to some garbage. 2 Being a variable all by itself, memory space would have to be emited by the compiler for "studFun". 3 The contents of this memory location should be considered random. 4 The compiler is free to do what it wants to, of course, as far as uninitialized data is concerned. 4.2 An optimizing compiler may not even generate memory space for "studFun" if "studFun" is declared but is never used. Hope the above will be helpful, – Aqeel Raza Dec 13 '17 at 06:48
  • Quite simply, you're creating a dangling pointer by overwriting the initial allocation on the line `stud[i] = new student[1]` with the `stud[i] = initiateStudent()`. call. In addition to that, you failed to mention at the end of the loop, all of the memory is not deallocated with a simple `delete [] stud;` call. – PaulMcKenzie Dec 13 '17 at 06:51
  • @PaulMcKenzie The reason I did not deallocate is because after this I need to pass this stud[i] to a different function – Spandyie Dec 13 '17 at 07:02
  • @Spandy -- I am speaking of at the very end of the program. The `delete [] stud;` does not deallocate all of the memory that was allocated. Second, the issue still stands -- this line: `stud[i] = new student[1];` should be removed (I guess that comment was directed @AqeelRaza.) – PaulMcKenzie Dec 13 '17 at 07:03