5

I'm trying to use Google's protobuf. I use a subMessage, I set it with random values and store the subMessage in the mainMessage. Here are example messages that fit my case :

message subMessage{
    required int32 val1= 1;
    required int32 val2= 2;
    required int32 val3= 3;
    required int32 val4= 4;
}

message mainMessage{
    required subMessage sub = 1;
}

My main :

int main (int argc,char** argv){
    subMessage sM;
    mainMessage mM;
    sM.set_val1(10);
    sM.set_val2(9);
    sM.set_val3(8);
    sM.set_val4(7);

I then tried :

mM.set_allocated_submessage(&sM);

but it result in a segmentation fault at the end of the program (Destruction of the object ?). The only way to solve it is to manually call mM.release_submessage();.

I also tried :

*mM.mutable_submessage()=subMessage;

I don't get why I have a segmentation fault as my program stops right after (Nothing accessing/writing on my subMessage or my mainMessage).
According to Google's documentation :

void set_allocated_foo(Bar* bar): Sets the Bar object to the field and frees the previous field value if it exists. If the Bar pointer is not NULL, the message takes ownership of the allocated Bar object and has_foo() will return true. Otherwise, if the Bar is NULL, the behavior is the same as calling clear_foo()

It seems that mainMessage has the ownership after set_allocated_foo(), but it results in a Segmentation fault. In the other hand, the mutable_foo() seems to duplicate the data. I would like to avoid duplicating datas as I'm going to run on Raspberry with low RAM, but the set_allocated_foo() is too mysterious for me for the moment. I don't get the difference between theses two...

Do you think I should use .set_allocated_foo() and manually release it, or use .mutable_foo() ?

Thanks.

PS : I'm aware of this topic about set_allocated_foo() but my problem isn't about deleting or not but more about rights on the pointers/datas and didn't help me solving my problem

Maskim
  • 382
  • 2
  • 4
  • 18

1 Answers1

12

Your code deletes LOCAL variable by delete operator. This is undefined behaviour. Your problem is that you are passing pointer to LOCAL variable, you should allocate subMessage by new then you can pass it into mainMessage.

int main()
{
    subMessage sM;  // [1]
    mainMessage mM; 
    sM.set_val1(10);
    sM.set_val2(9);
    sM.set_val3(8);
    sM.set_val4(7);
    mM.set_allocated_submessage(&sM); // [2]

    // calls dtors in reverse order of definitions of instances
    dtor on mM is called // [3]
    dtor on sM is called // [4]
}

In short:

[1] sM is created as local variable

[2] you are passing pointer to local variable sM, in this moment mM takes ownership of sM

[3] and because mM took ownership of sM it tries to delete sM in dtor by calling delete ptrToSm

[4] probably this point is never reached, app crashed

Solution:

 subMessage* sM = new ...;
 mM.set_allocated_submessage(sM);
rafix07
  • 20,001
  • 3
  • 20
  • 33
  • Ho great thanks, I didn't thought about this... It wasn't even a protobuf problem in the end... – Maskim Dec 06 '18 at 10:11
  • 1
    Do you know when I should use `.mutable_foo()` ? Now I'm wondering what's the use of this function... – Maskim Dec 06 '18 at 10:13
  • 4
    Suppose you have two messages, *Foo* and *Bar*, `message Bar { Foo foo;}`, `Bar b;` , member `foo` called on `b` returns `const Bar&` so you cannot modify any fields of `Bar` message. By calling `mutable_bar` you can do it. Use this method when you want to modify member. – rafix07 Dec 06 '18 at 10:27
  • 2
    I think the OP wanted to know the difference between `mutable_*` and `set_allocated_*`, dynamic allocation is always more expensive than stack allocation, so if `mutable_*` avoids that and returns already existing instance you can modify it's a better option. – Konrad Sep 25 '20 at 10:01
  • 1
    See relevant answer: https://stackoverflow.com/a/43327247/2828480 – Konrad Sep 25 '20 at 10:03