Adam, The point about the style guide is well-taken, I also understood that this is a deliberate divergence from the contract of std::map.
The function signature of at() claims it returns a reference but in practice it->second (at least in v3.2) returns by value. So I think the method may be depending on UB but like I said I don't claim to understand this completely. What I can report is that I observed a crash in my use case with a use-after free from doing a find and returning &iter->second as you suggest. I'll add that my code isn't quite as simple as TryGetValue(key)->field(); in that I pass the pointer along to other code when the stack probably looks different. Thanks, Mohamed On Wed, Feb 13, 2019 at 5:24 PM Adam Cozzette <[email protected]> wrote: > Google's C++ style avoids the use of exceptions, so I think that explains > why it does a GOOGLE_CHECK instead of throwing an exception. About your > use-after-free concern, I haven't read through too closely but I believe > everything is fine there because at() is returning a reference and not a > copy. You could probably simplify your TryGetValue() function by having it > do just a single lookup: my_map.find(key) returns an iterator so once > you've verified that it's not equal to end() you can return &iter->second. > > On Wed, Feb 13, 2019 at 12:19 PM Mohamed Koubaa <[email protected]> > wrote: > >> Hello, >> >> I'm using version 3.2. I have a google::protobuf::map<int, Msg> and >> would like to implement a function that returns either a pointer to value >> in the map given or a key or null if it does not exist. If I was using a >> std::map, I would do this. >> >> Msg const * Foo::TryGetValue(int key) const { >> try { >> auto& value = my_map.at(key); >> return &value; >> } catch (...) { >> return nullptr; >> } >> } >> >> Because protobuf's contract for "at" is different (it does a GOOGLE_CHECK >> rather than a throw if the item with the given key is different), I thought >> I could implement something similar like this, with the downside of doing >> two lookups rather than one. >> >> Msg const * Foo::TryGetValue(int key) const { >> auto iter = my_map.find(key); >> if (iter == my_map.end()) >> return nullptr; >> auto& value = my_map.at(key); >> return &value; >> } >> >> However, after peeking at the implementation of google::protobuf::map >> (and I don't claim to understand it completely) it seems that the "at" >> method does a find and then just returns iter->second which does a copy. >> So the client of TryGetValue will have used after free. Is there another >> way to do this? >> >> Thanks! >> Mohamed Koubaa >> Software Developer >> Ansys, Inc >> >> -- >> You received this message because you are subscribed to the Google Groups >> "Protocol Buffers" group. >> To unsubscribe from this group and stop receiving emails from it, send an >> email to [email protected]. >> To post to this group, send email to [email protected]. >> Visit this group at https://groups.google.com/group/protobuf. >> For more options, visit https://groups.google.com/d/optout. >> > -- You received this message because you are subscribed to the Google Groups "Protocol Buffers" group. To unsubscribe from this group and stop receiving emails from it, send an email to [email protected]. To post to this group, send email to [email protected]. Visit this group at https://groups.google.com/group/protobuf. For more options, visit https://groups.google.com/d/optout.
