void ChoreStack::add(int new_urgency_level, string new_job){
for(int i = 0; i < length_; i++){
Chore* temp_chore = chore_array_[i];
if(temp_chore->level == new_urgency_level){
temp_chore->joblist.append(new_job+"^");
}
chore_array_[i] = temp_chore;
free(temp_chore);
}
}
Hello, this is the part that Valgrind says "Invalid read of size 4". I have never learned anything related to memory allocation before. Could anyone please explain to me in details why there is a memory error and how to fix it? Thank you!
Here is my declaration:
class ChoreStack{
public:
ChoreStack();
~ChoreStack();
void initialize(int new_urgency_level);
void add(int new_urgency_level, string new_chore);
void erase(int erase_level);
void print();
void next();
int get_length();
private:
struct Chore{
int level;
string joblist;
};
Chore** chore_array_;
int length_;
string* split_joblist(string joblist);
int number_of_chores(string joblist);
};
ChoreStack::ChoreStack(): chore_array_(nullptr), length_(0){
}
ChoreStack::~ChoreStack(){
delete[] chore_array_;
}
void ChoreStack::initialize(int new_urgency_level){
delete[] chore_array_;
chore_array_ = new Chore*[new_urgency_level];
for(int i = 0; i < new_urgency_level; i++){
Chore* temp_chore = new Chore;
temp_chore->level = i+1;
temp_chore->joblist = "";
chore_array_[new_urgency_level-i-1] = temp_chore;
delete temp_chore;
}
length_ = new_urgency_level;
cout << "New priority list with levels 1-" << length_ << " initialized." << endl;
}
Here is the part related to ChoreStack::add() function in main():
int main(){
ChoreStack c;
string cmd_line;
string* cmd_ptr = new string[3];
bool initialized = false;
while(true){
cout << "chores> ";
getline(cin, cmd_line);
int cmd_num = 0;
if(cmd_line.find_first_not_of("ABCDEFGHIJKLMNOPQRSTUVWXYZabcdefghijklmnopqrstuvwxyz1234567890 ") != string::npos){
cout << "Error: invalid input." << endl;
} else {
int begin_i = 0, space_occurence = 0;
unsigned int i = 0;
while(i <= cmd_line.length()){
if(cmd_line[i] == ' ' || i == cmd_line.length()){
space_occurence++;
cmd_num++;
if(space_occurence == 3){
cmd_ptr[cmd_num-1] = cmd_line.substr(begin_i);
break;
} else {
cmd_ptr[cmd_num-1] = cmd_line.substr(begin_i, i - begin_i);
}
begin_i = i + 1;
}
i++;
}
string command = cmd_ptr[0];
if(command == "init"){
if(cmd_num == 1){
cout << "Error: the number of priority levels is missing." << endl;
} else if(cmd_num > 2){
cout << "Error: too much input for initializing." << endl;
} else {
if(cmd_ptr[1].find_first_not_of("1234567890") != string::npos){
cout << "Error: the number of priority levels must be an integer larger than 0." << endl;
} else if(stoi(cmd_ptr[1]) < 1){
cout << "Error: it is not possible to create a priority list with 0 or less levels." << endl;
} else {
c.initialize(stoi(cmd_ptr[1]));
initialized = true;
}
}
} else if(command == "add"){
if(!initialized){
cout << "Error: no priority list is initialized." << endl;
} else {
if(cmd_num == 1){
cout << "Error: priority level and chore description are missing." << endl;
} else if(cmd_ptr[1].find_first_not_of("1234567890") != string::npos
|| stoi(cmd_ptr[1]) < 1
|| stoi(cmd_ptr[1]) > c.get_length()){
cout << "Error: priority level must be an integer between 1-3." << endl;
} else if(cmd_num == 2){
cout << "Error: chore description is missing." << endl;
} else {
c.add(stoi(cmd_ptr[1]), cmd_ptr[2]);
}
}
}
Here is the error message:
invalid read of size 4:
1.ChoreStack::add(int,std::__cxx11::basic_string<char,std::char_traits<char>,std::allocator<char>>)
2.main
Address 0x5a9de70 is 0 bytes inside a block of size 40 free'd
1. operator delete(void*)
2. ChoreStack::initialize(int)
3. main
Block was alloc'd at
1. operator new(unsigned long)
2. ChoreStack::initialize(int)
3. main
and there are lot more errors in the same form of this one...
This is a classic access after free.
void ChoreStack::initialize(int new_urgency_level){
delete[] chore_array_;
chore_array_ = new Chore*[new_urgency_level];
for(int i = 0; i < new_urgency_level; i++){
Chore* temp_chore = new Chore;
temp_chore->level = i+1;
temp_chore->joblist = "";
chore_array_[new_urgency_level-i-1] = temp_chore;
delete temp_chore;
}
Look closely at this code. You create a pointer called temp_chore
. It points to an object you allocate with new
. You then copy the value of temp_chore
into chore_array_
. So now the array has a pointer to the object you allocated with new
.
But then you delete
the object you allocated. So now, chore_array_
has a pointer to an object you deleted. It is an error to attempt to dereference this pointer since the thing it points to no longer exists.
But then in add
, we have:
Chore* temp_chore = chore_array_[i];
if(temp_chore->level == new_urgency_level){
So temp_chore->level
is an attempt to access the level
member of the object temp_chore
points to. But you delete
d that object.
// Once you do this,
chore_array_[new_urgency_level-i-1] = temp_chore;
// this
delete temp_chore;
// does the same thing as this
delete chore_array_[new_urgency_level-i-1];
// because they're equal
You will find things much easier if you keep collections of values rather than collections of raw pointers.