Would calling the default parameter with `new` cause memory leak?

Question

I am trying to write a function that returns the center point of biggest detected object. No problem here. The issue is that I also want to return the foreground mask from parameters, if the user wants to use it.

In order to handle this situation, my solution is as following:

cv::Point detect(const cv::Mat &img, cv::Mat &mask = *new cv::Mat());

If mask parameter is specified, it can be used in main. Does RAII works in the other case, or would there be a memory leak?

Note: I know I can overload the function by 2 lines, but I want to know is it possible to do it properly by using default parameters, and not using pointers (input type is strict).

Another note: If cv::noArray() or a similar function can be used, that is totally okay.

Example usage:

char ch = 0;
while (ch != 27) // ESC is pressed
{
    cap >> img;
    if (img.empty())
        break;
    cv::Mat mask;
    cv::Point pt = detect(img, mask);
    // or pt = detect(img);
    cv::imshow("original", img);
    cv::imshow("foreground", mask);
    ch = cv::waitKey(1);
}

Impossible to say. The code could delete the underlying pointer, in which case there’s no leak. But that’s crufty. Don’t do it. — Pete Becker, Oct 11 '20 at 19:00
RAII is a design pattern that you have to implement, it's not something that C++ just enforces for free. I assume that if the user passes a value for `mask`, you don't delete the `cv::Mat` at the end of `detect`, which means you will indeed have a memory leak. Just take a pointer and use `cv::Mat* p_Mask = nullptr` or something if you want them to be able to call it without a mask. — Nathan Pierson, Oct 11 '20 at 19:01
a memory leak is not where you create something via new, but where you miss to delete it. — 463035818_is_not_an_ai, Oct 11 '20 at 19:22
Since the argument is a reference, the code still have a reference to the object, and can use the address-of operator `&` to get a pointer to it for `delete`. However there's another problem with that default arguments, and it's that lvalue references can't bind to rvalues which is what the result of `*new cv::Mat()` would be. — Some programmer dude, Oct 11 '20 at 19:24
@NathanPierson That is desired behaviour if `mask` is specified. I suppose there would be no problem in that case. Added an example code. — Burak, Oct 11 '20 at 19:37
@user2864740 I want to make it easy for user. If I choose to use a pointer, either `imshow("foreground", *mask);`, or a pointer to reference is required. — Burak, Oct 11 '20 at 19:40
@Someprogrammerdude `lvalue references can't bind to rvalues` that is what I get when I try to use `cv::Mat &mask = cv::Mat()`. If I remember correctly, VC++ does not complain, but Ubuntu GCC gives error. — Burak, Oct 11 '20 at 19:45
@Burak VC++ have a non-standard and non-portable extension that allows it. — Some programmer dude, Oct 11 '20 at 20:13

Etienne Laurin · Accepted Answer · 2020-10-12T13:31:12.607

1

Yes, that introduces a memory leak. You might instead consider using a tuple return value:

std::tuple<cv::Point, cv::Mat> detect(const cv::Mat &img);

or making mask a pointer:

cv::Point detect(const cv::Mat &img, cv::Mat *mask = nullptr);

You could also try to keep the same signature with a cast. This is ugly but it might work:

cv::Point detect(const cv::Mat &img, cv::Mat &mask = const_cast<cv::Mat&>(static_cast<cv::Mat const&>(cv::Mat())));

edited Oct 12 '20 at 13:31

answered Oct 11 '20 at 19:39

Etienne Laurin

6,731
2
27
31

I am aware of the first two solutions. Can you explain what it does in the third? – Burak Oct 11 '20 at 19:42
`error: ‘const’ qualifiers cannot be applied to ‘cv::Mat&’` [const reference to non-cost object is compilation error](https://stackoverflow.com/questions/39631317/what-is-the-difference-between-const-int-jj-and-int-const-jj#answer-39631367) – Burak Oct 11 '20 at 22:16
I've replaced the C-style casts with `const_cast` and `static_cast`. See https://stackoverflow.com/questions/35439619/invalid-initialization-of-non-const-reference-from-an-rvalue for another example – Etienne Laurin Oct 12 '20 at 13:32
With `const_cast`, now it works. This is what I want. Thanks! – Burak Oct 12 '20 at 19:12

score 1 · Answer 2 · answered Oct 12 '20 at 14:55

1

Ugly, but no memory leak:

cv::Point detect(const cv::Mat &img, cv::Mat &mask = *std::make_unique<cv::Mat>());

If the user does not provide a mask, one will be created. detect will write the mask back, which is then immediately destroyed. Using an overload will be more efficient as it can skip the needless update.

Alternatively, use cv::Mat* mask = nullptr. I don't know what you mean by "(input type is strict)." but for an experienced C++ programmer the cv::Mat* mask = nullptr pattern is clear: optional out parameter, no ownership transfer implied.

answered Oct 12 '20 at 14:55

MSalters

173,980
10
155
350

Well, it requires additional `memory` library and C++14. Most importantly, GCC version should be at least 5. `cmake_minimum_required(VERSION 3.14.0)` [source](https://stackoverflow.com/questions/24609271/errormake-unique-is-not-a-member-of-std) Those are very high prerequisites for such a simple function. – Burak Oct 12 '20 at 19:10
I don't use pointer because OpenCV `OutputArray` is used as like this. See [`Canny` Edge Detector](https://docs.opencv.org/3.4/da/d5c/tutorial_canny_detector.html) for example. – Burak Oct 12 '20 at 19:58
@Burak: Well, we're at C++17/20 now, and GCC 10, so C++14/GCC 5 didn't seem unreasonable. Either way, you can just use the underlying `std::unique_ptr` from C++11, or going _way_ back, Boost and C++98 from the previous century. – MSalters Oct 12 '20 at 21:21
The version of CMake installed by APT on Ubuntu 18.04 is currently 3.10.2. – Burak Oct 13 '20 at 08:04
It is interesting that `make_shared` requires C++11 whereas `make_unique` is C++14 standard. Shared is generally more applicable and I would expect the other way round. – Burak Dec 30 '20 at 10:56
@Burak: That's because `make_shared` was originally designed as an optimization. It usually co-locates the shared control block with the object itself, saving one allocation and reducing fragmentation. Since `unique_ptr` is a no-overhead pointer, there's no need for a similar optimization. `make_unique` was added for consistency and simplicity. – MSalters Dec 31 '20 at 11:28

Would calling the default parameter with `new` cause memory leak?

2 Answers2