C++ Do these constitute memory leaks?

Question

I've been working on a simple ray tracer and I'm having issues with running out of memory. I downloaded Visual Leak Detector for visual studio and it told me the following functions are causing memory leaks. I'm not sure why these would be considered leaks however:

Point* Point::copy() {

    Point* result = new Point(x, y, z);

    return result;
}

Point* Point::crossProduct(Point* other) {

    double crossX = y*(*other).getZ() - z*(*other).getY();
    double crossY = z*(*other).getX() - x*(*other).getZ();
    double crossZ = x*(*other).getY() - y*(*other).getX();

    Point* cross = new Point(crossX, crossY, crossZ);

    return cross;
}

Note I only discovered about copy constructors after creating and using the copy function shown here. If I was going to redo the project I'd use a copy constructor instead. Now when I use the functions I make sure to call "delete" on whatever variable I'm using. For example:

Point* n = AB.crossProduct(&AC);
...
delete n;

Am I wrong in thinking that this should handle any memory leak? Is Visual Leak Detector just unable to recognize the leak has been handled because its in a separate file?

Use smart pointers to avoid memory leaks. Although your assumption is correct. — Hemant Gangwar, Sep 23 '14 at 03:03
Why are you using `new` at all? You can fix any leaks there might be (and improve performance, probably by a massive amount) by returning simple objects like these by value. — Mike Seymour, Sep 23 '14 at 03:04

score 5 · Answer 1 · answered Sep 23 '14 at 03:16

Why not just return by value, and pass by const reference?

Point Point::copy() 
{
    return Point(x, y, z);
}

Point Point::crossProduct(const Point& other)
{
    double crossX = y * other.getZ() - z * other.getY();
    double crossY = z * other.getX() - x * other.getZ();
    double crossZ = x * other.getY() - y * other.getX();

    return Point(crossX, crossY, crossZ);
}

Of course your copy function is just a poor man's copy constructor/assignment operator, so use those instead:

Point::Point(const Point& other) 
    : x(other.x)
    , y(other.y)
    , z(other.z)
{
}

Point& Point::operator=(const Point& other) 
{
    x = other.x;
    y = other.y;
    z = other.z;
    return *this;
}

Of course, those copy/assignment functions are identical to the implicitly generated ones, so don't actually write them yourself. — Mike Seymour, Sep 23 '14 at 06:30

score 4 · Answer 2 · edited May 23 '17 at 10:26

The rule is:
Every dynamic memory allocation should have a corresponding deallocation.

Unless this rule is followed, you will end up with memory leaks. Atleast any of the memory leak detection tools will detect them so. There might be memory allocations which are never deallocated till end of lifetime of program but these instances will be very few. And you shouldn't be dabbling in to those unless you really understand the concepts well.

As for the simplest way to get memory allocations/deallocations this right is just to simply use Smart pointers.

Note: Yes your handling seems correct based on the code you have shown.

score 1 · Answer 3 · answered Sep 23 '14 at 03:01

1

Rather than managing memory manually, use smart pointers. This way, all the objects would be deallocated automatically, and you wouldn't have to worry about memory leaks.

answered Sep 23 '14 at 03:01

abacabadabacaba

2,662
1
13
18

C++ Do these constitute memory leaks?

3 Answers3