Change StunMessage::AddAttribute return type from bool to void.

webrtc/p2p/base/stun.cc

 /*
  *  Copyright 2004 The WebRTC Project Authors. All rights reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
 #include "webrtc/p2p/base/stun.h"
 
 #include <string.h>
 
 #include <memory>
 
 #include "webrtc/base/byteorder.h"
 #include "webrtc/base/checks.h"
-#include "webrtc/base/common.h"
 #include "webrtc/base/crc32.h"
 #include "webrtc/base/logging.h"
 #include "webrtc/base/messagedigest.h"
 #include "webrtc/base/stringencode.h"
 
 using rtc::ByteBufferReader;
 using rtc::ByteBufferWriter;
 
 namespace cricket {
 
@@ skipping 37 matching lines @@
 }
 
 bool StunMessage::SetTransactionID(const std::string& str) {
   if (!IsValidTransactionId(str)) {
     return false;
   }
   transaction_id_ = str;
   return true;
 }
 
-bool StunMessage::AddAttribute(StunAttribute* attr) {
+void StunMessage::AddAttribute(StunAttribute* attr) {
   // Fail any attributes that aren't valid for this type of message.
-  if (attr->value_type() != GetAttributeValueType(attr->type())) {
-    return false;
-  }
+  RTC_DCHECK_EQ(attr->value_type(), GetAttributeValueType(attr->type()));
+

[kwiberg-webrtc, 2017/02/01 10:40:17]

Old non-debug behavior for wrong attr type was to return early, leaking attr and
having the caller ignore the error. New non-debug behavior is to go ahead
anyway. What effect will this have?

[pthatcher2, 2017/02/01 22:15:06]

I think a DCHECK is sufficient.  I think there is no code which is passing in
the wrong type combination.  

It might be a good idea to use a std::unique_ptr to avoid the leak, even if
changing all calls sites would be a pain. 

[kwiberg-webrtc, 2017/02/02 08:39:00]

Well then. If no caller does the wrong thing, we're A-OK!

   attrs_->push_back(attr);
   attr->SetOwner(this);
   size_t attr_length = attr->length();
   if (attr_length % 4 != 0) {
     attr_length += (4 - (attr_length % 4));
   }
   length_ += static_cast<uint16_t>(attr_length + 4);
-  return true;
 }
 
 const StunAddressAttribute* StunMessage::GetAddress(int type) const {
   switch (type) {
     case STUN_ATTR_MAPPED_ADDRESS: {
       // Return XOR-MAPPED-ADDRESS when MAPPED-ADDRESS attribute is
       // missing.
       const StunAttribute* mapped_address =
           GetAttribute(STUN_ATTR_MAPPED_ADDRESS);
       if (!mapped_address)
@@ skipping 113 matching lines @@
   return AddMessageIntegrity(password.c_str(), password.size());
 }
 
 bool StunMessage::AddMessageIntegrity(const char* key,
                                       size_t keylen) {
   // Add the attribute with a dummy value. Since this is a known attribute, it
   // can't fail.
   StunByteStringAttribute* msg_integrity_attr =
       new StunByteStringAttribute(STUN_ATTR_MESSAGE_INTEGRITY,
           std::string(kStunMessageIntegritySize, '0'));
-  VERIFY(AddAttribute(msg_integrity_attr));
+  AddAttribute(msg_integrity_attr);
 
   // Calculate the HMAC for the message.
   ByteBufferWriter buf;
   if (!Write(&buf))
     return false;
 
   int msg_len_for_hmac = static_cast<int>(
       buf.Length() - kStunAttributeHeaderSize - msg_integrity_attr->length());
   char hmac[kStunMessageIntegritySize];
   size_t ret = rtc::ComputeHmac(rtc::DIGEST_SHA_1,
@@ skipping 40 matching lines @@
       rtc::GetBE32(fingerprint_attr_data + kStunAttributeHeaderSize);
   return ((fingerprint ^ STUN_FINGERPRINT_XOR_VALUE) ==
       rtc::ComputeCrc32(data, size - fingerprint_attr_size));
 }
 
 bool StunMessage::AddFingerprint() {
   // Add the attribute with a dummy value. Since this is a known attribute,
   // it can't fail.
   StunUInt32Attribute* fingerprint_attr =
      new StunUInt32Attribute(STUN_ATTR_FINGERPRINT, 0);
-  VERIFY(AddAttribute(fingerprint_attr));
+  AddAttribute(fingerprint_attr);
 
   // Calculate the CRC-32 for the message and insert it.
   ByteBufferWriter buf;
   if (!Write(&buf))
     return false;
 
   int msg_len_for_crc32 = static_cast<int>(
       buf.Length() - kStunAttributeHeaderSize - fingerprint_attr->length());
   uint32_t c = rtc::ComputeCrc32(buf.Data(), msg_len_for_crc32);
 
@@ skipping 620 matching lines @@
       digest, sizeof(digest));
   if (size == 0) {
     return false;
   }
 
   *hash = std::string(digest, size);
   return true;
 }
 
 }  // namespace cricket