5

I have simple messages:

message SmallValue {
    int32 val = 1;
}
message Value {
    int32 val1 = 1;
    int32 val2 = 2;
    SmallValue val3 = 3;
}
message SendMessage {
    int32 id = 1;
    oneof message {
        Value value= 2;
}

My piece of code:

// create new pointer for smallValue
SmallValue* smallValue = new SmallValue();
smallValue->set_val3(3);

// create new object value and set_allocated_val3
Value value;
value.set_val1(1);
value.set_val2(2);
value.set_allocated_val3(smallValue);

// create new object message and set_allocated_value
SendMessage message;
message.set_id(0);
message.set_allocated_value(&value);

// after some work, release value from message
message.release_value();

And my questions are:
1. After calling message.release_value() is it OK not to call delete &value; as I didn't create new pointer?
2. Will memory of smallValue will be deleted automatically along with value as I didn't call value.release_smallValue();?

// I'm a newbie to C++ as well as protobuf. Please do tell if something odd about my code.

Thanks!

Thanh Nhan
  • 453
  • 6
  • 17
  • I would be surprised if Google had a library that made memory management weird. I'd expect that you had to manage the memory yourself. That said, you only `delete` pointers that are allocated via `new`. `value` is not allocated via `new`; it is allocated on the stack. OTOH you should (almost) never have to explicitly type `new` or `delete`; you should use `std::unique_ptr` and `std::make_unique` – Justin Apr 07 '17 at 03:08
  • @Justin Thanks for the reference to the `std::unique_ptr` and `std::make_unique`. I'll be looking into it. – Thanh Nhan Apr 07 '17 at 03:19

1 Answers1

11

It is usually best to avoid using the set_allocated_* and release_* methods; those provide advanced memory-management features that you should not need unless you are really trying to optimize some performance-critical code.

You could rewrite your code like this to avoid having to worry about memory management as much:

SendMessage message;
message.set_id(0);
Value* value = message.mutable_value();
value->set_val1(1);
value->set_val2(2);
value->mutable_val3()->set_val(3);
Adam Cozzette
  • 1,396
  • 8
  • 9
  • Thanks for the advice! I will change to `mutable_*` instead of `new` and `set_allocated_*` but the `release_*` part is not mine to change. Which means `release_*` will be called after usage of message. Won't this cause memory leak as protobuf release ownership of pointer and I didn't delete it? – Thanh Nhan Apr 11 '17 at 08:11
  • Yes, if you call release then you must delete the pointer yourself to avoid a memory leak. Maybe the code calling `release_value()` should actually be calling `clear_value()`? – Adam Cozzette Apr 11 '17 at 14:35
  • Great point! It's actually is `clear_*` instead of `release_*`. Thanks a lot. – Thanh Nhan Apr 14 '17 at 08:13