0

This is the main Test File and cannot be changed and there are 3 files in total not including Header files. main.cpp, engine.cpp, ship.cpp. I have included a screenshot of my error as well but its the invalid read of size 8 error (memory leak) at the T5 part of the test in the ship.cpp file. It outputs the correct answer but obviously has a memory leak.My error pic

#include <iostream>
#include "Ship.h"
#include "Ship.h"
#include "Engine.h"
#include "Engine.h"

using namespace std;
using namespace sdds;

void printHeader(const char* title)
{
    char oldFill = cout.fill('-');
    cout.width(40);
    cout << "" << endl;

    cout << "|> " << title << endl;

    cout.fill('-');
    cout.width(40);
    cout << "" << endl;
    cout.fill(oldFill);
}

int main()
{
    {
        printHeader("T1: Testing Constants");

        cout << "TYPE_MAX_SIZE: " << sdds::TYPE_MAX_SIZE << endl;
        cout << "MIN_STD_POWER: " << sdds::MIN_STD_POWER << endl;
        cout << "MAX_STD_POWER: " << sdds::MAX_STD_POWER << endl;
        cout << endl;
    }

    {
        printHeader("T2: Testing Default Constructor");

        Ship invalid;
        invalid.display();
        invalid += Engine("D2", 2.1);
        cout << endl;
    }

    Engine engines[] = {
        Engine("V8", 4.4),
        Engine("V8", 5.0),
        Engine("Inline", 4.1),
        Engine("D3", 7.0),
        Engine("D0", 2.0),
        Engine("D1", 3.2),
    };

    {
        printHeader("T3: Testing Custom Constructor");
        
        Ship titanic("cruiser", engines, 6);
        titanic.display();
        cout << endl;
    }

    {
        printHeader("T4: Testing Conversion to Bool Operator");
        Ship invalid;
        Ship titanic("liner", engines, 1);

        if (invalid)
            cout << "1. Test Failed! Object should be invalid.\n";
        else
            cout << "1. Test succeeded!\n";

        if (titanic)
            cout << "2. Test succeeded!\n";
        else
            cout << "3. Test Failed! Object should be valid.\n";
        
        cout << endl;
    }

    {
        printHeader("T5: Testing += and < Operators");

        Ship titanic("liner", engines, 3);

        char type[]{ "D0" };
        while (titanic < sdds::MIN_STD_POWER)
        {
            type[1]++;
            cout << "Ship not up to standard. Required power: "
                 << sdds::MIN_STD_POWER << endl;
            titanic += Engine(type, 2.1);
        }

        titanic.display();

        if (sdds::MAX_STD_POWER < titanic)
            cout << "Too much power." << endl;
        else
            cout << "Ship doesn't exceed power regulation of: "
                 << sdds::MAX_STD_POWER << endl;
    }

    return 0;
}

This is my engine.cpp file

#include <iostream>
#include <iomanip>
#include <stdio.h>
#include <string.h>
#include "Engine.h"
using namespace sdds;
using namespace std;

namespace sdds {
Engine::Engine(){       // Default Empty Engine
    m_type[0] = '\0';
    m_size = 0.0;
}

Engine::Engine(const char* type, double size){  // Custom Engine
    strncpy(m_type, type, TYPE_MAX_SIZE);
    m_size = size;
}

double Engine::get() const{ // Getter for the size of the engine
    return m_size;
}

void Engine::display() const{   // Basic Display Function
    cout << m_size << " liters - " << m_type << endl;
    
}
}

This is my ship.cpp where the error is in the += overloaded operator function.

#include <iostream>
#include <cstring>
#include "Ship.h"

using namespace std;

namespace sdds {

Ship::Ship(){           // Default Ship (empty)
    m_type = nullptr;
    m_engines = nullptr;
    m_engCnt = 0;
}

Ship::Ship(const char* type, const Engine* engines, int engCnt){  // Custom Ship
    if (type != nullptr && engines != nullptr && engCnt > 0) {
        // creating a Valid Ship
        int len = (unsigned)strlen(type);
        m_type = new char[len + 1];
        strcpy(m_type, type);
        
        m_engines = new Engine[engCnt];
        
        for (int i = 0; i < engCnt; i++) {
            m_engines[i] = engines[i];
        }
        m_engCnt = engCnt;
        
    }else{
        m_type = nullptr; // Setting Ship to Empty State
        m_engines = nullptr;
        m_engCnt = 0;
    }
}

Ship::~Ship(){
    delete[] m_engines;
    delete[] m_type;
}

Ship::operator bool() const {
    // returning true if the ship is valid (not empty)
    return m_type != nullptr;
}

Ship& Ship::operator+=(Engine e){  // THIS IS WHERE THE ERROR IS <-----------------
    if (!*this) {
        cout << "The object is not valid! Engine cannot be added!" << endl;
    }else{
        Engine* tmp = new Engine[m_engCnt + 1];
        for (int i = 0; i <= m_engCnt; i++) {
            tmp[i] = m_engines[i];
        }
        tmp[m_engCnt++] = e;
        for (int i = 0; i <= m_engCnt; i++) {
            m_engines[i] = tmp[i];
        }
    }
    return *this;

}

double Ship::calculatePower() const {       // Multiplying the Engines Size * 5 to get the power(sum of all)
    double power = 0;
    for (int i = 0; i < m_engCnt; i++) {
        power += m_engines[i].get() * 5;
    }
    return power;
}

void Ship::display()const{  // Displaying the Ship
    if (*this) {
        streamsize dp = cout.precision(); // save default precision
        cout.precision(2); // change default precision
        cout << fixed; // enable fixed
        cout << m_type << " - " << calculatePower() << endl;
        for (int i = 0; i < m_engCnt; i++) {
            m_engines[i].display(); // Engines Display function
        }
        cout.unsetf(ios::fixed); // disable fixed
        cout.precision(dp); // restore default precision
    }else{
        cout << "No available data" << endl;
    }
}
bool Ship::operator<(double power) const{       // Comparing the passed in power with the power of the ship
    if (calculatePower() < power) {
        return true;
    }else{
        return false;
    }
}

bool operator<(double power, const Ship& theShip){  // Global < overloaded operator
    if (power < theShip.calculatePower()) {
        return true;
    }else{
        return false;
    }
}
}
Daboss2182
  • 25
  • 5
  • 2
    `m_engines[i]` accesses an index out of bounds on the last iteration of the loop, when `i == m_engCnt`. Also, you leak the block of memory that `tmp` points to. – Igor Tandetnik Jun 28 '20 at 22:27
  • "Invalid read" isn't a memory leak, it's a buffer overrun. – Nate Eldredge Jun 28 '20 at 22:27
  • @IgorTandetnik what would be the fix, because when i try to delete[] tmp it gives me errors aswell. New to dynamical memory allocation :( – Daboss2182 Jun 28 '20 at 22:30
  • You are probably still accessing an index out of bounds. The fix would be to stop doing that. – Igor Tandetnik Jun 28 '20 at 22:31
  • In your loops `for (int i = 0; i <= m_engCnt; i++)`, the `<=` should be `<`. That's what Igor is saying. – Nate Eldredge Jun 28 '20 at 22:33
  • @NateEldredge still the same error even after making the above changes. I do understand that i need to free the new tmp that I have created but not sure why it gives me the error. This is my professors pseudo code - - create a temp dynammic array of engines (m_engCnt + 1) - copy all the engines to the tmp array - add a new engine to the top of the tmp array - increase m_engCnt - delete the old array of engines - copy tmp array address to m_engines address. – Daboss2182 Jun 28 '20 at 22:44
  • I don't see "delete the old array of engines - copy `tmp` array address to `m_engines` address" part in the code shown. And you already have a buffer overrun in "copy all the engines to the `tmp` array" part. – Igor Tandetnik Jun 28 '20 at 22:48
  • @IgorTandetnik Thank you, got it fixed --> delete [] m_engines; m_engines = tmp; was the missing pieces and changed <= to <. Have Alignment issues now because its supposed to print [cruiser - 128.50] with 1 space in T3 and [liner - 99.00] with 2 spaces before the 99. I understand precision changes the points after the decimal but how would I get the precision before the decimal. – Daboss2182 Jun 28 '20 at 23:22
  • 1
    Don't use `new`/`delete`. Use container classes like `std::string` and `std::vector`. They are there for a reason – HAL9000 Jun 29 '20 at 00:40
  • @Daboss2182 -- Regardless of your fixes, this simple program has double-deletion errors: `int main() { Engine e("abc", 0.0); Ship s1("zzz", &e, 1); Ship s2 = s1; }` -- Both `s1` and `s2` will attempt to `delete[]` the same pointers when `main` exits. Your `Ship` class does not have correct copy-semantics due to the lack of a user-defined copy constructor and assignment operator. It violates the [rule of 3](https://stackoverflow.com/questions/4172722/what-is-the-rule-of-three). As indicated, use `std::vector` and `std::string`, and you don't worry about things like this. – PaulMcKenzie Jun 29 '20 at 01:15

0 Answers0