1

I have a function in which the callback receives an event holding a unique_ptr instace of data. I can retrieve the char* through event.data.get() which should give me the pointer but not ownership allowing the object to freely delete it. Now I have another function which takes a unique_ptr instance of otherdata and of course manages ownership so the function is free to release it. So I'm trying to take data from the callback and pass it into the function which takes otherdata safely.

void cbdata(const DataEvent& event){
                char* datatobewritten = event.data.get();
                stream.write(std::move(datatobewritten),event.length));
            }

The above example does seem to work but I'm not sure if it's the right way to do it. Is event giving up ownership? And if so, is it safe to give up the ownership?

And why does this result in a compiler error: Call to implicitly-deleted copy constructor of 'std::unique_ptr<char []>'?

void cbdata(const DataEvent& event){
                stream.write(event.data,event.length);
            }

If event.data gives me a unique_ptr and stream.write takes a unique_ptr then shouldn't the above code work fine? Or this

stream.write(std::move(event.data), event.length);

Sorry my C++ knowledge is really limited so unique_ptr and move semantics are pretty confusing.

Irelia
  • 3,407
  • 2
  • 10
  • 31
  • If you want our help in explaining a compiler error, it's always helpful to tell us the error. So please edit your question to include a copy-paste of the *full* and *complete* error. – Some programmer dude Sep 07 '19 at 11:24
  • Also, I recommend that you think of the smart pointers in terms of ownership. If you move a `std::unique_ptr` you also move the ownership of it. After `std::move(event.data)` then `event` no longer owns the data, and doesn't even have it anymore. – Some programmer dude Sep 07 '19 at 11:26
  • @Someprogrammerdude if an object is using a unique_ptr then is it expecting the ownership to be volatile and doesn't care who owns/releases it? – Irelia Sep 07 '19 at 11:28
  • 1
    `std::move(datatobewritten)` returns a `char*&&`, which is probably not doing what you think it does – Artyer Sep 07 '19 at 11:31
  • `std::unique_ptr` is to be used when you want a single owner. You can move the ownership but afterward, the source cannot access the data anymore. If you need to have multiple owners, then use `std::shared_ptr` instead. And why would you like to give ownership to the stream. If it is stand C++ stream, then it won't take ownership. – Phil1970 Sep 07 '19 at 13:22

2 Answers2

2

It is a bit unusual that your stream.write wants to take ownership of a fragment of an event that it is printing. It seems it takes ownership of a part of an event, while the caller remains the owner of the rest. After that call, an event objects remains incomplete, as the event.data was given away.

Moreover, your cbdata takes event through const&. It is a "borrow & do not change" semantic. As such, it should not give ownership of event or any part of it.

Some options:

  1. Pass ownership of the whole event into the stream, not just data. Your cbdata will need to aquire ownership first though.

  2. Do not give any ownership to stream. Usually streams are used just to present the data to the user and not manipulate them in any way.

  3. Change your event so that it can release its data permanently. You would need something like this:

    std::unique_ptr<DataType> Event::releaseData() {
        return std::move(this->data);
        //after the call, this->data is 'nullptr'!
    }
    

    of course in the rest of Event class, the case where data is nullptr must be supported.

    Note that this changes the event object, it is not a const function. Your cbdata won't work on const& event object.

  4. You can also copy the data into your stream. May be inefficient if data is big, but perfectly OK if DataType is copyable. You can do it like this:

    void cbdata(const DataEvent& event){
        stream.write(std::make_unique<DataType>(*event.data),event.length);
    }
    

    as with any other copy, you have to pay attention that when the copy is destroyed, only the copied data is actually deleted. Nothing that is shared between the original and the copy should be removed.

CygnusX1
  • 20,968
  • 5
  • 65
  • 109
  • Stream in this context is a handle , not necessarily just output. But the handle is to a socket and write would be writing the data. But the write takes a unique_ptr as well as char* so I think by accident I was calling the overloaded method which took char* but yes unfortunately the callback is const reference – Irelia Sep 07 '19 at 16:59
  • If the callback is const-reference, it means that the event is *immutable* and it cannot give ownership (of itself or a part of itself). If `stream` on the other hand requires to take ownership, then you need to reevaluate the program logic. You are not allowed to connect these two together - and for a reason! – CygnusX1 Sep 07 '19 at 18:29
  • So could I use the data from event to allocate a new unique_ptr and pass that into stream.write? and if so how can I? – Irelia Sep 07 '19 at 20:26
  • Ah, of course, you can *copy* the data! I'll add it to the list of options. – CygnusX1 Sep 08 '19 at 08:41
1

The stream.write() probably takes a const char *, if so you won't need to std::move() to pass data to it. std::move() is used when you need to change data ownership of an unique_ptr. But in this case data is own by event and we only temporarily let stream.write() access it.

void cbdata(const DataEvent& event){
    const char* datatobewritten = event.data.get();
    stream.write(datatobewritten, event.length);
}

If stream.write() takes a std::unique_ptr, so you'd need to std::move() the data from event.data to this function. After this move, event.data is in a "moved from" state and can't access the data anymore.

void cbdata(DataEvent& event){
    stream.write(std::move(event.data), event.length);
    // now 'event.data' owns nothing and holds a null pointer
}
frogatto
  • 28,539
  • 11
  • 83
  • 129
  • So my write takes both a char* and a unique-ptr and I was trying to call the one that took unique_ptr since its overloaded. One question I was wondering is how do I allocate a new unique_ptr then move data from the other into it. So two unique_ptr instances? – Irelia Sep 07 '19 at 16:55
  • I realised I could actually remove the const from the callback then stream.write(std::move(event.data)) compiles correctly. Thanks again. – Irelia Sep 08 '19 at 20:38