0

I want to read binary tree from file. I have written a method.

BinaryTree * FileService::read(BinaryTree * node) {
    basic_string<char> np = "nullptr";
    char * val = new char ();

    if (!fscanf(this->filePtr, "%s ", val)) {
        delete val;
        return node;
    }
    string str(val);
    if (str == np) {
        delete val;
        return node;
    }

    node = new BinaryTree();
    if (Helper::isFloatNumber(val)) {
        node->setNumericValue(stof(val));
    } else {
        node->setStrValue(val);
    }
    delete val; <----HERE SIGTRAP (Trace/breakpoint trap)
    node->setLeft(this->read(&node->getLeftReference()));
    node->setRight(this->read(&node->getRightReference()));

    return node;

When I execute my code, I get an error Process finished with exit code -1073740940 (0xC0000374) When I try to debug I receive this one SIGTRAP (Trace/breakpoint trap) where last delete val is. When I comment this line my code works well. But I think memory leak will be there in this case. Tell me please where am I wrong? And how can I change my code to avoid memory leak and exceptions?

I tried to move line delete val; before last return. It didn't fix. I tried to make condition

    if (val != nullptr) {
        delete val;
        val = nullptr;
    }

instead every delete val; result was the same.

Deadpool
  • 33
  • 7
  • 5
    `char * val = new char ();` -- Why are you allocating for a single character? And why is it necessary to allocate any data from the heap? – PaulMcKenzie Jan 23 '23 at 14:48
  • 1
    The best way to avoid memory leaks is to avoid using `new` and `delete` entirely (and in modern C++ that's usually easy) and instead use containers (and/or smart pointers - if you must). Naked `new`/`delete`, raw owning pointers and manual memory management in general has very little place in modern C++, outside very niche corners. – Jesper Juhl Jan 23 '23 at 14:51
  • 1
    `if (!fscanf(this->filePtr, "%s ", val))` -- Well, this is off, since you only allocated for one character. So there is no need to look any further in the code. – PaulMcKenzie Jan 23 '23 at 14:54
  • `new char()` ? are you sure? And that is before the advise not to use new/delete if you don't have to – Pepijn Kramer Jan 23 '23 at 14:54
  • the crash does not necessarily happen where the cause for the crash is. By removing the `delete val;` you did not solve the actual reason for the crash, you were just hiding it. – 463035818_is_not_an_ai Jan 23 '23 at 14:58
  • btw this is a cute example to understand the benefit of RAII. Whenever you see this pattern of some clean up (in this case its a `delete`) before every `return`, it should ring a bell. – 463035818_is_not_an_ai Jan 23 '23 at 15:01
  • @PaulMcKenzie I don't know how fscanf work, but my variable `val` must be this type only. In this case my file is read correctly and variable val receieve right result. – Deadpool Jan 23 '23 at 15:06
  • Any reason why you are still using (unsafe) fscanf? [C++ basic file I/O](https://www.learncpp.com/cpp-tutorial/basic-file-io/). And just a question, where are you learning C++ from? From what your code looks like it looks like it is an outdated source. – Pepijn Kramer Jan 23 '23 at 15:08
  • @PepijnKramer I am learning C++ from online courses and from internet. The reason of using fscanf is other methods (fread, out << ) is not read my file correctly. Thank you! I'll look other way to read my file. – Deadpool Jan 23 '23 at 15:15
  • @Deadpool *I am learning C++ from online courses and from internet.* -- Well, that's the first issue, and that is learning C++ not from peer-reviewed books, but from some course on the Internet. It has been well-documented how poor many of these online tutorials and websites are when it comes to teaching C++. Some use obsolete techniques, and outright spout incorrect information. – PaulMcKenzie Jan 23 '23 at 15:20
  • 2
    @Deadpool *In this case my file is read correctly and variable val receieve right result.* -- Undefined behavior, and it is *not* working correctly. You allocated for a single character, yet you are reading more than one byte of information into that buffer. You have memory corruption by doing this, and probably why later on, `delete` just goes haywire. – PaulMcKenzie Jan 23 '23 at 15:24
  • The issue is that a lot of what you can find online uses outdated examples, or just copy pastes from whatever is there. C++ evolves and new techniques are added over time to be able to write less buggy code. The site https://en.cppreference.com/w/ is good for reference it also has examples, and find that https://www.learncpp.com/ is very decent. After you've learned some C++ you also should look at [C++ core guidelines](https://isocpp.github.io/CppCoreGuidelines/CppCoreGuidelines) – Pepijn Kramer Jan 23 '23 at 15:41
  • There is also a [book list](https://stackoverflow.com/questions/388242/the-definitive-c-book-guide-and-list). In any case stay away from competitive coding sites. Avoid `using namespace std;` and be careful when looking at examples with `#include ` (that is NOT a standard C++ header file) – Pepijn Kramer Jan 23 '23 at 15:44

0 Answers0