I have 3 files in total and I get the output I want but I am getting a memory leak error when compile With Valgrind (It's a tool for checking is there any memory leak in code)
main.cpp
#include<iostream>
#include<iomanip>
#include "Car.h"
using namespace std;
using namespace sdds;
int main()
{
const int num_cars = 6;
Car carInventory[num_cars] = {
{},
{"suv", "volvo", "xc90"},
{"Truck", "Ford", "F 150", 2021, 105, 55000},
{"Sedan", "BMW", "M550i", 2022, 101, 91000},
{"Truck", "Tesla", "Cybertruck", 2021, 102, 65000},
{"Sedan", "BMW", "M550i"}
};
if (carInventory[2].setInfo("SUV", "Volvo", "XC90", 2019, 109, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo(nullptr, "Volvo", "XC90", 2019, 109, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo("SUV", nullptr, "XC90", 2019, 109, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo("SUV", "Volvo", nullptr, 2019, 109, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo("SUV", "Volvo", "XC90", 1934, 109, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo("SUV", "Volvo", "XC90", 2019, 1, 80000).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
if (carInventory[1].setInfo("SUV", "Volvo", "XC90", 2019, 109, 0).isValid()) {
cout << "Information was set correctly!" << endl;
}
else {
cout << "Information was set incorrectly!" << endl;
}
cout << setw(60) << "----- Testing printInfo function -----" << endl << endl;
cout << "| Type | Brand | Model | Year | Code | Price |" << endl;
cout << " ------------ ------------------ ------------------ ------ ------ ----------- " << endl;
carInventory[4].printInfo();
cout << endl << endl;
cout << setw(60) << "----- Car Inventory Information -----" << endl << endl;
cout << "| Type | Brand | Model | Year | Code | Price |" << endl;
cout << " ------------ ------------------ ------------------ ------ ------ ----------- " << endl;
print(carInventory, num_cars);
if (has_invalid(carInventory, num_cars)) {
cout << endl;
cout << setfill('*') << setw(60) << "*" << endl;
cout << "* WARNING: There are invalid data in the inventory! *" << endl;
cout << setfill('*') << setw(60) << "*" << endl;
}
if (has_similar(carInventory, num_cars)) {
cout << endl;
cout << setfill(' ') << setw(60) << " " << endl;
cout << " WARNING: There are similar entries in the inventory! " << endl;
cout << setfill(' ') << setw(60) << " " << endl;
}
return 0;
}
Car.h
#ifndef SDDS_CAR_H
#define SDDS_CAR_H
namespace sdds{
class Car{
private:
char *m_type;
char *m_brand;
char *m_model;
int m_year;
int m_code;
double m_price;
public:
Car();
Car(const char *type, const char *brand, const char *model, int year = 2022, int code = 100, double price = 1);
~Car();
void resetInfo();
Car& setInfo(const char *type, const char *brand, const char *model,int yera,int code, double price);
void printInfo() const;
bool isValid() const;
bool isSimilarTo(const Car& car) const;
};
bool has_similar(const Car car[], const int num_cars);
bool has_invalid(const Car car[], const int num_cars);
void print(const Car car[], const int num_cars);
}
#endif
Car.cpp
#include <bits/stdc .h>
#include "Car.h"
using namespace std;
namespace sdds{
void Car::resetInfo(){
m_type = nullptr;
m_brand = nullptr;
m_model = nullptr;
m_year = 0;
m_code = 0;
m_price = 0;
}
Car::Car(){
resetInfo();
}
Car::Car(const char* type, const char* brand, const char* model, int year, int code, double price){
if(type != nullptr && brand != nullptr && model != nullptr && year > 0 && code > 0 && price > 0){
m_type = new char[strlen(type) 1];
strcpy(m_type, type);
m_brand = new char[strlen(brand) 1];
strcpy(m_brand, brand);
m_model = new char[strlen(model) 1];
strcpy(m_model, model);
m_year = year;
m_code = code;
m_price = price;
}
else{
cout<<"any time here\n";
resetInfo();
}
}
Car::~Car(){
delete[] m_type;
delete[] m_brand;
delete[] m_model;
m_type = nullptr;
m_brand = nullptr;
m_model = nullptr;
}
Car& Car::setInfo(const char *type,const char *brand, const char *model,int year,int code,double price){
this->resetInfo();
if(type == nullptr || brand == nullptr || model == nullptr || year < 1990 || code <= 99 || price <= 0){
// this->resetInfo();
return *this;
}
else{
m_type = new char[strlen(type) 1];
strcpy(m_type, type);
m_brand = new char[strlen(brand) 1];
strcpy(m_brand, brand);
m_model = new char[strlen(model) 1];
strcpy(m_model, model);
m_year = year;
m_code = code;
m_price = price;
}
return *this;
}
void Car::printInfo()const{
//cout << "| " << m_type << setw(8) << "| " << m_brand << setw(14) << "| " << m_model << setw(9) << " | " << m_year << setw(3) << "| " << m_code << setw(4) << " | " << m_price << setw(6) << " |" << endl;
if(m_type != nullptr && m_brand != nullptr && m_model != nullptr && m_year >= 1990 && m_code > 99 && m_price > 0){
printf("| %-10s | %-15s | %-15s | %-4d | M | %8.2lf |\n",m_type,m_brand,m_model,m_year,m_code,m_price);
}
}
bool Car::isValid()const{
//return m_type != nullptr && m_brand != nullptr && m_model != nullptr && m_year > 0 && m_code > 0 && m_price > 0;
if(m_type != nullptr && m_brand != nullptr && m_model != nullptr && m_year >= 1990 && m_code >= 100 && m_price > 0){
//cout<<m_type<<" "<<m_brand<<" "<<m_model<<" "<<m_year<<" "<<m_code<<" "<<m_price<<endl;
return true;
}
else{
//cout<<m_type<<" "<<m_brand<<" "<<m_model<<" "<<m_year<<" "<<m_code<<" "<<m_price<<endl;
return false;
}
}
bool Car::isSimilarTo(const Car& other)const{
if(m_type == other.m_type && m_brand == other.m_brand && m_model == other.m_model && m_year == other.m_year){
return true;
}
else{
return false;
}
return false;
}
bool has_similar(const Car car[],const int num_cars){
for(int i = 0; i < num_cars; i ){
for(int j = i 1; j < num_cars; j ){
if(car[i].isSimilarTo(car[j])){
return true;
}
}
}
return false;
}
bool has_invalid(const Car car[], const int num_cars){
for(int i = 0; i < num_cars; i ){
if(!car[i].isValid()){
return true;
}
}
return false;
}
void print(const Car car[], const int num_cars){
for(int i = 0; i < num_cars; i ){
car[i].printInfo();
}
}
}
Here is the output I want And I got it
Information was set correctly!
Information was set incorrectly!
Information was set incorrectly!
Information was set incorrectly!
Information was set incorrectly!
Information was set incorrectly!
Information was set incorrectly!
----- Testing printInfo function -----
| Type | Brand | Model | Year | Code | Price |
------------ ------------------ ------------------ ------ ------ -----------
| Truck | Tesla | Cybertruck | 2021 | 102 | 65000.00 |
----- Car Inventory Information -----
| Type | Brand | Model | Year | Code | Price |
------------ ------------------ ------------------ ------ ------ -----------
| SUV | Volvo | XC90 | 2019 | 109 | 80000.00 |
| Sedan | BMW | M550i | 2022 | 101 | 91000.00 |
| Truck | Tesla | Cybertruck | 2021 | 102 | 65000.00 |
| Sedan | BMW | M550i | 2022 | 100 | 1.00 |
************************************************************
* WARNING: There are invalid data in the inventory! *
************************************************************
WARNING: There are similar entries in the inventory!
But here is the memory leak error
==86467==
==86467== HEAP SUMMARY:
==86467== in use at exit: 32 bytes in 6 blocks
==86467== total heap usage: 19 allocs, 13 frees, 72,806 bytes allocated
==86467==
==86467== 4 bytes in 1 blocks are definitely lost in loss record 1 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C2C: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:27)
==86467== by 0x401229: main (main_prof.cpp:23)
==86467==
==86467== 5 bytes in 1 blocks are definitely lost in loss record 2 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C98: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:31)
==86467== by 0x401229: main (main_prof.cpp:23)
==86467==
==86467== 5 bytes in 1 blocks are definitely lost in loss record 3 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C61: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:29)
==86467== by 0x40126D: main (main_prof.cpp:23)
==86467==
==86467== 6 bytes in 1 blocks are definitely lost in loss record 4 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C61: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:29)
==86467== by 0x401229: main (main_prof.cpp:23)
==86467==
==86467== 6 bytes in 1 blocks are definitely lost in loss record 5 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C2C: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:27)
==86467== by 0x40126D: main (main_prof.cpp:23)
==86467==
==86467== 6 bytes in 1 blocks are definitely lost in loss record 6 of 6
==86467== at 0x4C2AC38: operator new[](unsigned long) (vg_replace_malloc.c:433)
==86467== by 0x400C98: sdds::Car::Car(char const*, char const*, char const*, int, int, double) (Car.cpp:31)
==86467== by 0x40126D: main (main_prof.cpp:23)
==86467==
==86467== LEAK SUMMARY:
==86467== definitely lost: 32 bytes in 6 blocks
==86467== indirectly lost: 0 bytes in 0 blocks
==86467== possibly lost: 0 bytes in 0 blocks
==86467== still reachable: 0 bytes in 0 blocks
==86467== suppressed: 0 bytes in 0 blocks
==86467==
==86467== ERROR SUMMARY: 6 errors from 6 contexts (suppressed: 0 from 0)
So this is the memory leak issue I am facing can anyone please help me to solve this
CodePudding user response：
When you call setInfo() you don't delete the strings you allocated in the constructor and the variables are assigned new values, the previous pointers are dangling and cannot be deleted - hence the memory leak
In cpp I would suggest to use
std::string, there is no need in allocating raw pointers in that case - and these memory leaks are a good lesson why.Code review wise - if you do decide to use pointers, the destructor and
resetInfo()have much in common and you can useresetInfo()as part of the destructor.
Good luck!
CodePudding user response：
I've had a cursory look at your code and it seems to me that you're stuck between two worlds - the old C world and the new(er) C world. If I recall correctly, the approved method in C for a null pointer is simply to use a zero (0), rather than 'nullptr' like you've got here. I'd be very suspicious of that part where you're assigning nullptr to a pointer (3, in fact). If that was dynamically allocated memory, you're doing nothing to reclaim it. Also, is there any good reason why you're using C-style character arrays rather than the standard library's strings?