Why do my pointers cause a segfault?

Question

#include <iostream>
#include <stdio.h>
#include <string.h>
#include <vector>
using namespace std;
  struct student
  {
    char name[50];
    char lname[50];
    int id;
    float GPA;
  };
void toAdd(vector<student*> *plist);
void toDelete(vector<student*>* plist);
void toPrint(vector<student*>* plist);
int main(){
  vector<student*> list;
  vector<student*>* plist = &list;
  char input[80];
  bool running = true;
  while (running == true){
    cout << "What would you like to do?" << endl;
    cin.getline (input,80);
    if(strcmp (input, "ADD") == 0){
      toAdd(plist);
    }
    else if(strcmp (input, "DELETE") == 0){
    }
    else if(strcmp (input, "PRINT") == 0){
    }
    else{
      cout << "That is not a valid response!" << endl;
        }
  }
}

void toAdd(vector<student*> *plist){
  student* stu;
  cout << "What a test!" << endl;
  cout << "First Name: ";
  cin.getline(stu->name,20);
  cout << "Last Name: ";
  cin.getline(stu->lname,20);
  cout << "ID: ";
  cin.getline(stu->id);
  cout << "GPA: ";
  cin.getline(stu->GPA);
  plist->push_back(stu);
}
void toDelete(){

}
void toPrint(){
}

I don't understand what I'm doing wrong. I've been going over this code for hours, and really need some help. I get the error "Segmentation Fault(core dumped)" When I run the code and try to input a name. I feel like this is probably really simple, but none of the online explanations are helping me. :(

You don't need that many stars. Prefer string to character arrays. — Ron, Nov 12 '17 at 03:28
`toAdd` dereferences an uninitialized pointer `stu`, whereupon your program exhibits undefined behavior. — Igor Tandetnik, Nov 12 '17 at 03:28
The plist that is passed into the add function isn't even initialized when you use it. And why does the list variable even exist in main? it doesn't seem like its being used in the reset of your code. — CAMD_3441, Nov 12 '17 at 03:33
One: There's no need for `plist`, learn about *references*. Two: There's no need to store *pointers* to your structure in the vector. Three: Learn about [`std::string`](http://en.cppreference.com/w/cpp/string/basic_string). Four: Learn about [`std::getline`](http://en.cppreference.com/w/cpp/string/basic_string/getline). Five: Either start paying more attention in class, or [get better beginners books](http://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). — Some programmer dude, Nov 12 '17 at 03:35

score 3 · Answer 1 · answered Nov 12 '17 at 03:34

3

You dereference a pointer which was not initialized. Before you call

cin.getline(stu->name,20);

You need to allocate its memory first using new

student* stu = new student;

answered Nov 12 '17 at 03:34

Zamrony P. Juhara

5,222
2
24
40

Terrible advice. – Aluan Haddad Nov 12 '17 at 03:34
This approach will cause memory leaks in short order. The method should just return a `student` – Aluan Haddad Nov 12 '17 at 03:37
1

Then OP needs to make sure to properly free its memory when its done with `plist` – Zamrony P. Juhara Nov 12 '17 at 03:39
At least use `std::unique_ptr` but really there is no need for a pointer. – Aluan Haddad Nov 12 '17 at 03:40
It is recommended, that if a pointer is needed to use smart pointers as pointed out by Aluan. – HaroldSer Nov 12 '17 at 03:42
1

@AluanHaddad The question is not about whether or not there is need for a pointer. The question is why the pointer is not working. People still need to learn how to use pointers even if their are other tools available. – Galik Nov 12 '17 at 03:42
@Galik Using `new` is considered a bad practice even when using pointer types because it is not clear when and how the function expects memory to be deleted. The issue is that the calling code doesn't even know that a pointer is created let alone that it is safe to delete. – Aluan Haddad Nov 12 '17 at 03:44
@AluanHaddad How does a student learn about pointers and new without using pointers and new? I get that it is good to mention better practices but we also should just answer the question. – Galik Nov 12 '17 at 03:45
They can use `std::unique_ptr` which is a pointer type that automatically cleans up. But technically, yes, `new` might be called for, but it is an advanced technique. If you use `new` it should generally be inside a class constructor where the destructor calls `delete` on the same pointer field. That would be an appropriate way to learn. – Aluan Haddad Nov 12 '17 at 03:48

HaroldSer · Answer 2 · 2017-11-12T03:36:43.870

2

Seg fault is usually caused when you are trying to access an area of memory location(to read or write) that is not allowed. In below line, within toAdd() method:

cin.getline(stu->name,20);

stu->name which is the equivalent for: (*stu).name is trying to dereference a pointer which was not initialized.

Furthermore, it is recommended to use smart pointers(Wrapper for raw(regular) pointers), since it allows you to manage the memory allocated for that pointer more efficiently. You can either choose unique or shared smart pointer.

edited Nov 12 '17 at 03:36

answered Nov 12 '17 at 03:32

HaroldSer

2,025
2
12
23

You are right but you should explain why the program is fundamentally flawed and can be rewritten to be shorty and not even use this technique. – Aluan Haddad Nov 12 '17 at 03:33

Why do my pointers cause a segfault?

2 Answers2