top | item 38485297

(no title)

bjorn2k | 2 years ago

C++ is not easy.

But if you think about ownership you can spot the bug in a proper code-review. Smart pointers are about ownership, where a unique_ptr has unique (one owner) ownership. If you create a second owner, which happens in fun1, then that second owner is cleaning up before (at the end of scope -> RAII), it is cleaned up in by its first owner (resulting in a double free)

A guideline which is often used, is:

pass by reference -> not transferring ownership

pass by raw pointer -> transferring ownership

So having a function with a raw pointer argument flags transfer of ownership, but isn't necessarily so. The transfer really happens on line 44.

A unique_ptr actually guards you from that, because it is move only (cannot be copied). Using a std::unique_ptr on the interface of fun1 would have caught the problem at compile time, because an explicit move would have been needed to tranfer ownership, which would have put the original object in an valid but unspecified state. This would result in the original owner not cleaning up the resource, and therefore not resulting in a double free.

By using the get function on the unique pointer you actually remove the guards of having one owner.

There are a lot of things wrong with this code, which after being solved I don't see how you would need a pointer at all. Otherwise, you need to know about the guidelines of using smart pointers.

So my conclusion after reading this code is: This looks like C code with some C++ features sprinkled in. If you want to write C++ you need to know a lot about the language and its guidelines. Thinking about ownership is hard but necessary in any language. Having the ownership correct would have made it easy to add smart pointers. If the ownership is not clear, using raw pointers will not save you.

discuss

order

No comments yet.