Segmentation error with vector of struct

Hi everyone!

I'm having a whale of a time trying to figure out why, or rather, how to avoid getting the segmentation error after the getline input on line 29 (output of line 30 is never reached due to segmentation error).

Line 28 was me trying to fill index 0 of the toDoList.lists vector with *something* in hopes that I could avoid the segmentation error this way, but of course that didn't work...

Line 27 is from a suggested answer I saw on StackOverflow, but I must say that I don't fully understand it; it looks like I'm filling a vector which is ALREADY of the type List with...an element of type List???

I'm assuming that I'm referring to an empty index, or an index that doesn't exist, but I'm unsure which it is (if that is the case).

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
struct Task{
  string name;
  Date deadline;
  bool isDone;
  int time;
};

struct List{
  string name;
  vector<Task> tasks;
};

struct Project{
  int id;
  string name;
  string description;
  vector<List> lists;
};

void addList(Project &toDoList, int &listnumber){
    bool match=false;
    int i=0;
    
    do{
        match=false;
        cout << "Enter list name: " << endl;
        (toDoList.lists).push_back(List());
        (toDoList.lists[listnumber].name) = "0";
        getline(cin, toDoList.lists[listnumber].name);
        cout << 'k'; // this is a marker
        for(i=0; (i==listnumber) || (match==true); i++){
            if((toDoList.lists[listnumber].name)==(toDoList.lists[i].name)){
                match=true;
            }
        }
        if((toDoList.lists[listnumber].name).empty()){
            error(ERR_EMPTY);
        }
        else if(match==true){
            error(ERR_LIST_NAME);
        }
    }
    while(((toDoList.lists[listnumber].name).empty()) || (match==true));

    listnumber++;
}

int main(){
  Project toDoList;
  toDoList.id=1;
  char option;
  int listnumber=0;
  
  do{
    showMainMenu();
    cin >> option;
    cin.get();
    
    switch(option){
      case '1': editProject(toDoList);
                break;
      case '2': addList(toDoList, listnumber);
                break;
      case '3': deleteList(toDoList);
                break;
      case '4': addTask(toDoList);
                break;
      case '5': deleteTask(toDoList);
                break;
      case '6': toggleTask(toDoList);
                break;
      case '7': report(toDoList);
                break;
      case 'q': break;
      default: error(ERR_OPTION);
    }
  }while(option!='q');
  
  return 0;    
}


EDIT: Just added the main function for clarification on what the program is meant to be.
Last edited on
You have a lot of extraneous parentheses in your code.
There's no need to compare match to true. Just test it directly. (i.e, if (match))
You need to use an std::endl (or std::flush) to flush your debug output, otherwise it may not appear even though the segfault was after that line.

listnumber looks suspicious. It doesn't seem to be needed. How about something like this:

1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
struct List {
    string name;
    vector<Task> tasks;

    List(const string& n) : name(n) { }
};

struct Project {
    int id;
    string name;
    string description;
    vector<List> lists;
};

void addList(Project& toDoList) {
    cout << "Enter list name:\n";
    string name;
    getline(cin, name);
    bool duplicate = false;
    for (const auto& list: toDoList.lists)
        if (list.name == name) {
            duplicate = true;
            break;
        }
    if (duplicate)
        error(ERR_LIST_NAME); // I don't know what this does
    else
        toDoList.lists.emplace_back(name); // assuming ctor that takes name
}

Last edited on
Hello noobstudent,

I would start back at line 20 and look at what the value of the 2 variables are being passed to the function.

Your error may be at line 29, but it does not mean that it starts there.

Andy

P.S. A few blank lines would go a long way at making the code easier to read. Consider this:
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
struct Task
{
    string name;
    Date deadline;
    bool isDone;
    int time;
};

struct List
{
    string name;
    vector<Task> tasks;
};

struct Project
{
    int id;
    string name;
    string description;
    vector<List> lists;
};

void addList(Project &toDoList, int &listnumber)
{
    bool match = false;
    int i = 0;

    do
    {
        match = false;

        cout << "Enter list name: " << endl;

        (toDoList.lists).push_back(List());

        (toDoList.lists[listnumber].name) = "0";

        getline(cin, toDoList.lists[listnumber].name);

        cout << 'k'; // this is a marker

        for (i = 0; (i == listnumber) || (match == true); i++)
        {
            if ((toDoList.lists[listnumber].name) == (toDoList.lists[i].name))
            {
                match = true;
            }
        }

        if ((toDoList.lists[listnumber].name).empty())
        {
            error(ERR_EMPTY);
        }
        else if (match == true)
        {
            error(ERR_LIST_NAME);
        }
    } while (((toDoList.lists[listnumber].name).empty()) || (match == true));

    listnumber++;
}

Sorry my IDE changes the way the {}s are used.
@dutch - Yes, I figured a lot of my parentheses are unnecessary (I assume you're referring in particular to the ones in my conditions?). I get a little anxious about accidentally producing errors or the code not being 'explicit' enough, but I will try going without.

Also the 'k' does indeed print after flushing with 'endl', oops!

'listnumber' is supposed to be keeping track of the indices of the 'lists' vector so that I can then also traverse the vector and compare to the new list name being input, as well as making sure the new list names don't overwrite another.

Thanks for the code suggestion; I don't yet quite understand the term constructors, so I need to do some reading to understand what's going on in your suggestion.

@Handy Andy - You're so right about the spaces, I usually am better about that but was forgetting to do it this time around. Much more legible that way!

Will be mulling over the code a bit longer, see if I come to some sort of resolution...
As lists is a vector, std::find_if() can be used. So (not tried) :

1
2
3
4
5
6
7
8
9
10
11
12
13
void addList(Project& toDoList) {
    string name;
 
    cout << "Enter list name:\n";
    getline(cin, name);

    auto duplicate {std::find_if(lists.begin(), lists.end(), [&name] (const auto& l) {return name == l.name;}) != lists.end()};

    if (duplicate)
        error(ERR_LIST_NAME); // I don't know what this does
    else
        toDoList.lists.emplace_back(name); // assuming ctor that takes name
}

Last edited on
Hello noobstudent,

No worries about the code, but if you get into the habit of writing the code so that you can easily read it it just becomes a habit you do with out thinking about it. At first you may need to slow down a bit until good habits become natural.

It is always to post enough code that can be compiled and tested. It dos not have to be the whole program, sometimes it helps, but enough to demonstrate the problem.

Because of what is not there I am not sure how to set up to test the function.

Andy
Hello noobstudent,

I finally managed to get something to compile and run and this is what I found.

At first when I asked about the value of "toDoList" in line 20 As I suspected it it empty at least as far as the vector is concerned.

The code:
1
2
3
4
5
toDoList.lists).push_back(List());

//toDoList.lists[listnumber].name) = "0";

getline(cin, toDoList.lists[listnumber].name);

Does take care of this problem. Line 3 is not necessary. There is no need to give "name" a value just to overwrite the value on the next line.

The segmentation fault comes from your for loop.

The first time in the function "listnumber" has a value of (0) zero. in the for loop:
1
2
3
4
5
6
7
for (i = 0; (i == listnumber) || (match == true); i++)
{
    if ((toDoList.lists[listnumber].name) == (toDoList.lists[i].name))
    {
        match = true;
    }
}

This starts with "i == listnumber" which is true, so the rhs is never checked and you enter the loop.

At this point: if ((toDoList.lists[0].name) == (toDoList.lists[0].name)) which evaluates to true, so you enter the if statement and set "match" to true.

The second time through the loop you have: (i == listnumber) || (match == true). Here the lhs is false, so it checks the rhs which is true and will always be true until it creates a segmentation fault to crash the program. BTW "match" never changes here until you get back to the top of the do/while loop.

Inside the for loop: if ((toDoList.lists[0].name) == (toDoList.lists[1].name)), but since this is the first time through the function the vector "lists" only has 1 element, so the rhs of == is trying to check an element that does not exist yet.

Not sure what you want here: while (toDoList.lists[listnumber].name.empty() || (match == true));? The lhs should always be false because by the time that you reach here "name" will have something in it. The rhs will always be true because the value of "match" has not changed yet, so the whole condition will evaluate to true, always.

I am thinking that B4 the closing } of the do/while loop you do need to move "listnumber++;" inside the do/while loop. Unless the function is to run only 1 time then you will need to change the while condition to end the loop.

Like seeplus said I do not know what "error(ERR_EMPTY);" and "error(ERR_LIST_NAME);" are or what to do with them. The first I kind of understand because the "std::getline()" will allow you to press "Enter" and create an empty string, but maybe you would want this check after the "std::getline()" to check if an empty string was entered, print out an error message for not allowing an empty string and allow the user to enter something B4 continuing.

Andy
Topic archived. No new replies allowed.