No compilation errors and, it seems, no syntax errors -- but garbage is displayed?

Question

Working code fixed and below

Here's the whole .cpp. Any thoughts why it's printing out pure garbage when the overload void display(const Kingdom kingdoms[], size_t count) function is called?

File Kingdom.h
#ifndef KINGDOM_H
#define KINGDOM_H
#include <cstdlib>
// TODO: sict namespace
namespace sict
{
    // TODO: define the structure Kingdom in the sict namespace

        struct Kingdom {
            char m_name[32];
            int m_population;
        };
        // TODO: declare the function display(...),
        //         also in the sict namespace
        void display(const Kingdom& pKingdom);
        void display(const Kingdom kingdoms[], size_t count);
}


#endif

File Kingdom.cpp
#include <iostream>
#include <cstdlib>
#include "Kingdom.h"
using namespace std;
// TODO: the sict namespace
namespace sict
{
    // TODO:definition for display(...)

    void display(const Kingdom& pKingdom) {
        cout << pKingdom.m_name << ", " << "population " << pKingdom.m_population << endl;
    }

    void display(const Kingdom kingdoms[], size_t count) {
        cout << "------------------------------" << endl;
        cout << "Kingdoms of SICT" << endl;
        cout << "------------------------------" << endl;
        int pop = 0;
        for (size_t i = 0; i < count; i++) {
            cout << i + 1 << ". ";
            display(kingdoms[i]);
                pop += kingdoms[i].m_population;
        }
        cout << "------------------------------" << endl;
        cout << "Total population of SICT: " << pop << endl;
        cout << "------------------------------";
    }
}


    File main.cpp
    #include <iostream>
    #include <cstring> //for size_t definition
    #include <vector>
    #include "Kingdom.h"

    using namespace std;
    using namespace sict;

    void read(Kingdom&);

    int main() {
        int count = 0; // the number of kingdoms in the array

        // TODO: declare the pKingdom pointer here (don't forget to initialize it)
        Kingdom *pKingdom = nullptr;
        cout << "==========\n"
            << "Input data\n"
            << "==========\n"
            << "Enter the number of Kingdoms: ";
        cin >> count;
        cin.ignore();

        if (count < 1) return 1;

        // TODO: allocate dynamic memory here for the pKingdom pointer
        pKingdom = new Kingdom[count];
        for (int i = 0; i < count; ++i) {
            cout << "Kingdom #" << i + 1 << ": " << endl;
            // TODO: add code to accept user input for Kingdom i
            read(pKingdom[i]);
        }
        cout << "==========" << endl << endl;

        // testing that "display(...)" works
        cout << "------------------------------" << endl
            << "The 1st Kingdom entered is" << endl
            << "------------------------------" << endl;
        display(pKingdom[0]);
        cout << "------------------------------" << endl << endl;

        // expand the array of Kingdoms by 1 element
        count = count + 1;
        Kingdom *cpy_pKingdom = nullptr;
        // TODO: allocate dynamic memory for count + 1 Kingdoms
        cpy_pKingdom = new Kingdom[count];
        // TODO: copy elements from original array into this newly allocated array
        for (int i = 0; i < count; i++) {
            cpy_pKingdom[i] = pKingdom[i];
        }
        // TODO: deallocate the dynamic memory for the original array
        delete[] pKingdom;
        // TODO: copy the address of the newly allocated array into pKingdom pointer
        pKingdom = cpy_pKingdom;
        // add the new Kingdom
        cout << "==========\n"
             << "Input data\n"
             << "==========\n";
        cout << "Kingdom #" << count << ": " << endl;
             // TODO: accept input for the new element in the array
             read(pKingdom[count - 1]);
        cout << "==========\n" << endl;

        // testing that the overload of "display(...)" works
        display(pKingdom, count);
        cout << endl;

        // TODO: deallocate the dynamic memory here
        //delete[] pKingdom;
        //delete[] cpy_pKingdom;
        getchar();
        return 0;
    }

    // read accepts data for a Kingdom from standard input
    //
    void read(Kingdom& pkingdom) {
        cout << "Enter the name of the Kingdom: ";
        cin.get(pkingdom.m_name, 32, '\n');
        cin.ignore(2000, '\n');
        cout << "Enter the number of people living in " << pkingdom.m_name << ": ";
        cin >> pkingdom.m_population;
        cin.ignore(2000, '\n');

This doesn’t address the question, but do you really need the extra stuff that `std::endl` does? `’\n’` ends a line. — Pete Becker, Sep 23 '17 at 21:07

Peter · Answer 1 · 2017-09-24T00:43:03.433

The obvious offending lines (there may be others) are

cpy_pKingdom = new Kingdom[count + 1];
cpy_pKingdom = pKingdom;
delete[] pKingdom;
pKingdom = cpy_pKingdom;

The assignment cpy_pKingdom = pKingdom is a pointer assignment, and does not copy any array. It also discards the result of the new expression, and means that both cpy_pKingdom and pKingdom have the original value that pKingdom had.

The delete [] pKingdom then releases memory pointed to by pKingdom which is also (because of the preceding assignment cpy_pKingdom = pKingdom) the memory pointed to by cpy_Kingdom.

The net effect of these four lines is that the new Kingom[count + 1] is leaked (it is allocated, never released, but also inaccessible) and that both pKingdom and cpy_pKingdom point at deallocated memory.

Even worse, the next statement that does anything with pKingdom is

read(pKingdom[count + 1]);

which reads data from a file into the non-existent pKingdom[count + 1]. And bear in mind that - even if the previous code had make pKingdom point at the memory allocated by new Kingdom[count + 1] - this would be writing past the end of that memory. Also undefined behaviour.

And then we get to

 display(pKingdom, count);

which will access that same non-existent memory to print out the data there.

Displaying the garbage here is a minor consequence of one or more of the preceding instances of undefined behaviour in your code.

No compilation errors and, it seems, no syntax errors -- but garbage is displayed?

1 Answers1