NULL-check the entry in NonAllocatingMap before setting on it.

NULL-check the entry in NonAllocatingMap before setting on it.

Using just an assert will still cause a crash in a release build.

BUG=http://code.google.com/p/chromium/issues/detail?id=238757
R=mark@chromium.org

Committed: https://code.google.com/p/google-breakpad/source/detail?r=1174

[Mark Mentovai]

https://breakpad.appspot.com/593003/diff/1/src/common/simple_string_dictionary.h
File src/common/simple_string_dictionary.h (right):

https://breakpad.appspot.com/593003/diff/1/src/common/simple_string_dictionar...
src/common/simple_string_dictionary.h:194: assert(entry);
You seem to want to be able to handle the case where you run out of space in the
map now. That shouldn’t be a release-only thing. You should get rid of this
assert and fix the test and documentation accordingly. You can’t have something
be fatal in debug mode and explicitly tolerable in release mode.

[rsesek]

https://breakpad.appspot.com/593003/diff/1/src/common/simple_string_dictionary.h
File src/common/simple_string_dictionary.h (right):

https://breakpad.appspot.com/593003/diff/1/src/common/simple_string_dictionar...
src/common/simple_string_dictionary.h:194: assert(entry);
On 2013/05/08 15:54:26, Mark Mentovai wrote:
> You seem to want to be able to handle the case where you run out of space in
the
> map now. That shouldn’t be a release-only thing. You should get rid of this
> assert and fix the test and documentation accordingly. You can’t have
something
> be fatal in debug mode and explicitly tolerable in release mode.

Done.

[Mark Mentovai]

LGTM with one more word.

https://breakpad.appspot.com/593003/diff/4001/src/common/simple_string_dictio...
File src/common/simple_string_dictionary.h (right):

https://breakpad.appspot.com/593003/diff/4001/src/common/simple_string_dictio...
src/common/simple_string_dictionary.h:160: // operation fails.
“fails” is funny for something that doesn’t return a success value. Does “fail”
mean “crash?”

“fails silently” makes it sound more like what you mean.

[rsesek]

https://breakpad.appspot.com/593003/diff/4001/src/common/simple_string_dictio...
File src/common/simple_string_dictionary.h (right):

https://breakpad.appspot.com/593003/diff/4001/src/common/simple_string_dictio...
src/common/simple_string_dictionary.h:160: // operation fails.
On 2013/05/08 16:04:03, Mark Mentovai wrote:
> “fails” is funny for something that doesn’t return a success value. Does
“fail”
> mean “crash?”
> 
> “fails silently” makes it sound more like what you mean.

"silently fails" just to be different.

[Mark Mentovai]

not lgtm

[Mark Mentovai]

just kidding. LGTM.

[rsesek]

Committed patchset #3 manually as r1174 (presubmit successful).