Issue 426 in protobuf: repeated_field.h : some ::Mutable(..) methods do check validity of index, others don't. Why?

[prot...@googlecode.com]

Status: New
Owner: liuj...@google.com
Labels: Type-Defect Priority-Medium

New issue 426 by hpgisler...@bluewin.ch: repeated_field.h :
some ::Mutable(..) methods do check validity of index, others don't. Why?
http://code.google.com/p/protobuf/issues/detail?id=426

What steps will reproduce the problem?
1. myMessage.set_repeated_field(0,"some str message");

Note: without first calling myMessage.add

What is the expected output? What do you see instead?
I would expect a fatal log message (as is the case with
myMessage.repeated_field(0) if this field does not exist.


What version of the product are you using? On what operating system?
2.4.1
KEIL ARM Cortex M3 cross compiler on Windows 7

Please provide any additional information below.
Why is it that some methods do an index check and others don't?

e.g. repeated_field.h

line 663:
template <typename TypeHandler>
inline typename TypeHandler::Type*
RepeatedPtrFieldBase::Mutable(int index) {
GOOGLE_DCHECK_LT(index, size());
return cast<TypeHandler>(elements_[index]);
}

vs.

line 863:
template <typename Element>
inline Element* RepeatedPtrField<Element>::Mutable(int index) {
return RepeatedPtrFieldBase::Mutable<TypeHandler>(index);
}

The latter one will produce an undefined situation (pointer to nowhere)
when called e.g. by a method like this one created by .proto compiler:

inline void MbSendMessage::set_receivers(int index, const char* value) {
receivers_.Mutable(index)->assign(value);
}

Where as the first version would (rightly) produce a fatal log message.

[prot...@googlecode.com]

Updates:
Status: Invalid

Comment #1 on issue 426 by xiaof...@google.com: repeated_field.h :

some ::Mutable(..) methods do check validity of index, others don't. Why?
http://code.google.com/p/protobuf/issues/detail?id=426

The check is a DCHECK, which will be a nop in release build. You shouldn't
rely on it emitting a fatal error for you.
Adding them may make the crash report more readable but omitting them is
also perfectly fine.