I need to check a char array to see if it is valid or not, it could be a nullptr, but not a unsigned pointer, and the safe state is has is first element = '\0'
here is my code that i tried to make work for valgrind but did not succeed
1 2 3 4 5 6 7 8
bool Menu::isEmpty(void) const {
//Return true if title is in safestate
bool rtn = false;
if (m_title != nullptr) {
rtn = m_title[0] == '\0'; <<<<Valgrind doesn't like this
}
return rtn;
}
here is the error
==106406== Invalid read of size 1
==106406== at 0x401ACD: sdds::Menu::isEmpty() const (Menu.cpp:230)
==106406== by 0x401AA7: sdds::Menu::operator bool() const (Menu.cpp:223)
==106406== by 0x40114E: main (ms1_MenuTester.cpp:68)
==106406== Address 0x5a24040 is 0 bytes inside a block of size 16 free'd
==106406== at 0x4C2BB8F: operator delete[](void*) (vg_replace_malloc.c:651)
==106406== by 0x4017BF: sdds::Menu::~Menu() (Menu.cpp:114)
==106406== by 0x401126: main (ms1_MenuTester.cpp:65)
==106406== Block was alloc'd at
==106406== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==106406== by 0x40169D: sdds::Menu::setTitle(char const*) (Menu.cpp:72)
==106406== by 0x4016F5: sdds::Menu::Menu(char const*, int) (Menu.cpp:82)
==106406== by 0x400D0B: main (ms1_MenuTester.cpp:25)
==106406==
You can't. There's no test you can perform on a non-null pointer that will tell you if it's valid or invalid. All you can is always correctly initialize all pointers you use.
its a design error i've come to accept, the pointer goes into a scope by value and gets deleted by destructor when exiting scope, then the next call from client code (which i cant change) checks the isEmpty() funciton. Trying to figure out a way i can implement a variable that stays alive through the destructor of sorts.
You need to implement a copy constructor that performs a full copy of the object, that way when the copy is destructed only the copy's pointers are released, not the original's.
Example:
1 2 3 4 5 6 7 8 9 10 11 12 13
class A{
int *p;
public:
A(int n): p(newint(n)){}
A(const A &other): p(newint(*other.p)){}
A &operator=(const A &other){
*this->p = *other.p;
return *this;
}
~A(){
deletethis->p;
}
}
Menu& Menu::operator=(const Menu& menu) {
//Check if menu has title
if (menu) {
//Check if menu has items
if (menu.m_menuCount > 0) {
//Check if we have items
if (m_menuCount > 0) {
//Delete our items
for (int i = 0; i < m_menuCount; i++) {
delete m_menuItem[i];
}
}
//and allocate for new items
for (int i = 0; i < menu.m_menuCount; i++) {
//Add item updates count aswell
add(menu.m_menuItem[i]->m_item);
}
}
else {
//Delete our items
for (int i = 0; i < m_menuCount; i++) {
delete m_menuItem[i];
m_menuItem[i] = nullptr;
}
//Reset count
m_menuCount = 0;
}
//Copy indentation
m_indentation = menu.m_indentation;
//Clear our title
delete[] m_title;
//Set new title
setTitle(menu.m_title);
}
else {
//Set to safe state
setSafe();
m_menuCount = 0;
m_indentation = 0;
}
//Return object
return *this;
}
This is an assignment operator, not a copy constructor. It will be very similar, but it is not the same.
The variable menu is a reference, not a pointer, so line 3 is meaningless. Generally you cannot have a null reference.
You never set m_menuCount in the case when menu.m_menyCount > 0 (unless that's done inside "add".)
I'm not sure how m_menuItem is defined. I don't think you are properly cleaning up the table itself.
If you used std::string, std::vector and std::unique_ptr, all of your news and deletes would go away and you would have a much more robust, safe project.