Read a file with only char pointers in c++

Question

I need to read a file content into certain objects, unfortunately i'm not at liberty to use std::string and thereby have to use char pointers. However when i do this i get weird signs coming directly from the memory, the solution to that is however not working. So i rebuilded by using getline directly from the istream instead of the std library, but the same happens. How can i read the file correctly without making use of std::string.

PortsContainer game::ParsePort(std::istream& stream)
{
    PortsContainer ports;

    bool passFirstRow = false;

    char* portLine = new char[1000000];
    int i = 0;
    while (!stream.eof())
    {
        if (!stream)
            throw std::system_error(Error::STREAM_ERROR);

        if (portLine[0] == '\0' || portLine == nullptr || portLine[0] == '#')
            continue;

        std::stringstream ss(portLine);

        if (!passFirstRow) {
            char* name = new char[100];
            while (!ss.eof()) {
                ss.getline(name, sizeof name, ';');
                Port* port = new Port();
                //port->name = const_cast<char*>(name);
                port->name = const_cast<char*>(name);
                ports.addItem(port);
            }

            passFirstRow = true;
        } else {
            i++;
        }

        if (!stream)
            throw std::system_error(Error::STREAM_ERROR);
    }

    return ports;
}

PortsContainer game::ParsePort(std::istream& stream, std::error_code& errorBuffer)
{
    try
    {
        return ParsePort(stream);
    }
    catch (std::system_error exception)
    {
        errorBuffer = exception.code();
    }
}

PortsContainer game::GetAvailablePorts()
{
    PortsContainer ports;

    std::ifstream stream("./ports.csv");

    std::error_code errorBuffer;
    ports = ParsePort(stream, errorBuffer);
    if (errorBuffer)
        return PortsContainer();

    return ports;
}

First I recommend you read [Why is iostream::eof inside a loop condition considered wrong?](https://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong). Then you should remember that `char` strings in C++ are really called ***null-terminate** byte strings*. The null-terminator (not to be confused with a null pointer) is important. Also remember that the memory you allocate will *not* be initialized, and even reading it leads to *undefined behavior*. Lastly, you probably have a few memory leaks here and there. — Some programmer dude, Oct 25 '17 at 17:53
_i'm not at liberty to use std::string_ YAIT (yet another incompetent teacher) — , Oct 25 '17 at 18:02
It doesn't make sense that you are not allowed to use `std::string` but you are allowed to use `std::stringstream` which is based on `std:: string` — Remy Lebeau, Oct 25 '17 at 18:04
Why is this marked -1, the question is clear. @Someprogrammerdude i know chars are called null-terminate byte strings. That why couldn't use std::string and just convert it directly to with `c_str()` so im not working with a string. the memory leak are not in here because that part is processed else where. it get cleaned up when it should be, it's a getter after all. everything that is imported get initialized, to what far are you referring exectly? @ manni, the maximum capacity of the var type name has. — Multi-Cab, Oct 25 '17 at 18:08
And where do you free `portLine`? And where do you fill or read into `portLine`? — Some programmer dude, Oct 25 '17 at 18:10
@RemyLebeau the rule is to not use containers, `std::string` is considered as. stringstream is likely the same but i'm first processing the logic before handle the type rules, problem is with std::string instead of char* it is working when using std::getline instead `var.getline(dest, size)`. — Multi-Cab, Oct 25 '17 at 18:11
@Someprogrammerdude the filereading is working, i know that because with std::string it is working. but i will add just to make the way it works clear. @ manni66, might be but that's not solution to the issue here, even if i change it to 100 (the given size) or something else it's still not gonna work. — Multi-Cab, Oct 25 '17 at 18:15
How could the reading be working since you never even read from `stream`? — Some programmer dude, Oct 25 '17 at 18:17
@Someprogrammerdude magic? seriously I don't know how it works but whenever i put a breakpoint and i see the content of the file i'm glad. this it literally the code i have, so there nothing i'm hiding from you. this is how it works, the stuff behind it i don't know. — Multi-Cab, Oct 25 '17 at 18:32
@Multi-Cab: the code you showed never reads ANYTHING from `stream`, so there is no possible way you can be seeing the file data in your buffers — Remy Lebeau, Oct 25 '17 at 18:35
@Someprogrammerdude maybe it's the difference between ifstream and istream but i can garantee you there is no other part. it's like this and it compiles correctly, breakpoints and hover are showing the correct content. — Multi-Cab, Oct 25 '17 at 18:44

Remy Lebeau · Accepted Answer · 2017-10-25T20:01:27.150

You are not populating portLine with any data. In fact, you are not reading any data from stream at all.

You are misusing eof(). The eofbit flag is not updated until AFTER a read operation is attempted first. So you must read before elavulating eof().

You are leaking your portLine and name buffers. And worse, since you are not allowed to use std::string, that implies that the Port::name member is a char* pointer, which means you have (potentially) multiple Port objects pointing at the same physical buffer in memory. If Port tries to free that buffer later on, such as in its destructor, you are going to have memory errors.

Try something more like this instead:

PortsContainer game::ParsePort(std::istream& stream)
{
    if (!stream)
        throw std::system_error(Error::STREAM_ERROR);

    PortsContainer ports;

    bool passFirstRow = false;

    // better would be to use std::unique_ptr<char[]>, std::vector<char>,
    // or std::string instead so the memory is freed automatically ...
    char *portLine = new char[1000000];

    int i = 0;

    do
    {
        if (!stream.getline(portLine, 1000000))
        {
            delete[] portLine; // <-- free the buffer for line data...
            throw std::system_error(Error::STREAM_ERROR);
        }

        if ((stream.gcount() == 0) || (portLine[0] == '#'))
            continue;

        if (!passFirstRow)
        {
            std::istringstream iss(portLine);

            // better would be to use std::unique_ptr<char[]>, std::vector<char>,
            // or std::string instead so the memory is freed automatically ...
            char* name = new char[100];

            while (iss.getline(name, 100, ';'))
            {
                if (iss.gcount() == 0) continue;

                Port *port = new Port();
                port->name = name; // <-- assumes ownership is transferred!
                ports.addItem(port);
                name = new char[100]; // <-- have to reallocate a new buffer each time!
            }
            delete[] name; // <-- free the last buffer not used...
            passFirstRow = true;
        } else {
            ++i;
        }
    }
    while (!stream.eof());

    delete[] portLine; // <-- free the buffer for line data...

    return ports;
}

PortsContainer game::ParsePort(std::istream& stream, std::error_code& errorBuffer)
{
    try
    {
        return ParsePort(stream);
    }
    catch (const std::system_error &exception)
    {
        errorBuffer = exception.code();
        return PortsContainer(); // <-- don't forget to return something!
    }
}

PortsContainer game::GetAvailablePorts()
{
    std::ifstream stream("./ports.csv");
    std::error_code errorBuffer;
    return ParsePort(stream, errorBuffer); // <-- no need to check errorBuffer before returning!
}

however, I strongly suggest you using the STL smart pointers to help ensure safer memory management:

PortsContainer game::ParsePort(std::istream& stream)
{
    if (!stream)
        throw std::system_error(Error::STREAM_ERROR);

    PortsContainer ports;

    bool passFirstRow = false;

    // since you are using std::error_code, that means you are
    // using C++11 or later, so use std::unique_ptr to ensure
    // safe memory management...
    std::unique_ptr<char[]> portLine(new char[1000000]);

    int i = 0;

    do
    {
        if (!stream.getline(portLine.get(), 1000000))
            throw std::system_error(Error::STREAM_ERROR);

        if ((stream.gcount() == 0) || (portLine[0] == '#'))
            continue;

        if (!passFirstRow)
        {
            std::istringstream iss(portLine.get());

            // use std::unique_ptr here, too...
            std::unique_ptr<char[]> name(new char[100]);

            while (iss.getline(name.get(), 100, ';'))
            {
                if (iss.gcount() == 0) continue;

                // use std::unique_ptr here, too...
                std::unique_ptr<Port> port(new Port);
                port->name = name.release(); // <-- assumes ownership is transferred!
                                             // better to make Port::name use std::unique_ptr<char[]> and then std::move() ownership of name to it...
                ports.addItem(port.get());
                port.release();

                name.reset(new char[100]); // <-- have to reallocate a new buffer each time!
            }
            passFirstRow = true;
        } else {
            ++i;
        }
    }
    while (!stream.eof());

    return ports;
}

Though, using std::string would be the best option:

PortsContainer game::ParsePort(std::istream& stream)
{
    if (!stream)
        throw std::system_error(Error::STREAM_ERROR);

    PortsContainer ports;

    bool passFirstRow = false;
    std::string portLine;
    int i = 0;

    while (std::getline(stream, portLine))
    {
        if (portLine.empty() || (portLine[0] == '#'))
            continue;

        if (!passFirstRow)
        {
            std::istringstream iss(portLine);
            std::string name;

            while (std::getline(iss, name, ';'))
            {
                if (name.empty()) continue;

                std::unique_ptr<Port> port(new Port);
                port->name = name; // <-- make Port::name be std::string as well!
                ports.addItem(port.get());
                port.release();
            }
            passFirstRow = true;
        } else {
            ++i;
        }
    }

    if (!stream)
        throw std::system_error(Error::STREAM_ERROR);

    return ports;
}

Thanks for your support and explanation i got it working for the most part now at the end it crashes but that is in different section i gotta fix. In relation to the string, i find it ridiculous aswell but nothing i can do about it unfortunately. the container thing i get because it makes sure the student understands the logic behind pointers and stack & heap and that kind of stuff but the std::string thing was just "isn't that a container aswell? oh yeah you're right, you can't use that aswell". so no logic behind it and doesn't teach anything at all besides anger management. — Multi-Cab, Oct 25 '17 at 18:37
The original answers was more related. as my previous comment suggest, this part of the course wants to let the student/me learn memory management, so smart pointers are not allowed. For some reason I get memory leaks but I will be able to solve those. Again thanks for your solution(s) provided. — Multi-Cab, Oct 25 '17 at 19:04
I reverted the code back to using raw `char*` pointers, but I kept the other code in as extra suggestions. — Remy Lebeau, Oct 25 '17 at 20:02
++ for effort. I would say that emphasising manual memory management in a C++ course is rather contrary to the point of the language... to me, that's something that should be taught later, and perhaps only if the student expresses an interest in using those facilities, because for some reason the RAII classes of the stdlib don't fulfil their needs. For most people, the stdlib does what's needed, and they should be taught more useful things, IMO. — underscore_d, Oct 25 '17 at 20:05
@underscore_d: yes, many agree that the STL should be taught first as it is core to the C++ standard, and then lower-level techniques should be taught later on as more advanced topics like algorithm design, optimization,etc. — Remy Lebeau, Oct 25 '17 at 20:14

Read a file with only char pointers in c++

1 Answers1