Fix "value possibly truncated" warnings on MSVC, net/spdy/ edition.

net/spdy/spdy_framer.cc

Index: net/spdy/spdy_framer.cc
diff --git a/net/spdy/spdy_framer.cc b/net/spdy/spdy_framer.cc
index f1b1de3d1396c7204bf870e220e325ffa9f06a9a..30191664ddefc1288803eb5ffd276a79467f30c8 100644
--- a/net/spdy/spdy_framer.cc
+++ b/net/spdy/spdy_framer.cc
@@ -689,7 +689,7 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
 
   bool is_control_frame = false;
 
-  uint16 control_frame_type_field =
+  int control_frame_type_field =
       SpdyConstants::DataFrameType(protocol_version());
   // ProcessControlFrameHeader() will set current_frame_type_ to the
   // correct value if this is a valid control frame.
@@ -715,7 +715,9 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
       }
       // We check control_frame_type_field's validity in
       // ProcessControlFrameHeader().
-      successful_read = reader->ReadUInt16(&control_frame_type_field);
+      uint16 control_frame_type_field_uint16;
+      successful_read = reader->ReadUInt16(&control_frame_type_field_uint16);
+      control_frame_type_field = control_frame_type_field_uint16;
     } else {
       reader->Rewind();
       successful_read = reader->ReadUInt31(&current_frame_stream_id_);
@@ -735,17 +737,15 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
     bool successful_read = reader->ReadUInt24(&length_field);
     DCHECK(successful_read);
 
-    uint8 control_frame_type_field_uint8 =
-      SpdyConstants::DataFrameType(protocol_version());

[Peter Kasting, 2014/11/25 22:51:00]

This initialization is redundant with the one above the conditional.

+    uint8 control_frame_type_field_uint8;
     successful_read = reader->ReadUInt8(&control_frame_type_field_uint8);
     DCHECK(successful_read);
     // We check control_frame_type_field's validity in
     // ProcessControlFrameHeader().
     control_frame_type_field = control_frame_type_field_uint8;
-    is_control_frame = (protocol_version() > SPDY3) ?
-      control_frame_type_field !=
-      SpdyConstants::SerializeFrameType(protocol_version(), DATA) :
-      control_frame_type_field != 0;
+    is_control_frame = control_frame_type_field !=

[Peter Kasting, 2014/11/25 22:51:00]

This is just a shorter version of the original statement.

+        ((protocol_version() > SPDY3) ?
+            SpdyConstants::SerializeFrameType(protocol_version(), DATA) : 0);
 
     if (is_control_frame) {
       current_frame_length_ = length_field + GetControlFrameHeaderSize();
@@ -838,7 +838,7 @@ size_t SpdyFramer::ProcessCommonHeader(const char* data, size_t len) {
   return original_len - len;
 }
 
-void SpdyFramer::ProcessControlFrameHeader(uint16 control_frame_type_field) {
+void SpdyFramer::ProcessControlFrameHeader(int control_frame_type_field) {
   DCHECK_EQ(SPDY_NO_ERROR, error_code_);
   DCHECK_LE(GetControlFrameHeaderSize(), current_frame_buffer_length_);
 
@@ -1191,7 +1191,7 @@ void SpdyFramer::WriteHeaderBlock(SpdyFrameBuilder* frame,
                                   const SpdyMajorVersion spdy_version,
                                   const SpdyHeaderBlock* headers) {
   if (spdy_version < SPDY3) {
-    frame->WriteUInt16(headers->size());
+    frame->WriteUInt16(static_cast<uint16>(headers->size()));
   } else {
     frame->WriteUInt32(headers->size());
   }
@@ -1270,7 +1270,7 @@ static void WriteLengthZ(size_t n,
   char buf[4];
   DCHECK_LE(length, sizeof(buf));
   for (unsigned i = 1; i <= length; i++) {
-    buf[length - i] = n;
+    buf[length - i] = static_cast<char>(n);
     n >>= 8;
   }
   WriteZ(base::StringPiece(buf, length), clas, out);
@@ -1489,7 +1489,7 @@ size_t SpdyFramer::ProcessControlFrameBeforeHeaderBlock(const char* data,
           }
           const bool has_priority =
               (current_frame_flags_ & HEADERS_FLAG_PRIORITY) != 0;
-          uint32 priority = 0;
+          SpdyPriority priority = 0;
           if (protocol_version() > SPDY3 && has_priority) {
             // TODO(jgraettinger): Process dependency rather than ignoring it.
             reader.Seek(kPriorityDependencyPayloadSize);
@@ -2471,7 +2471,7 @@ SpdySerializedFrame* SpdyFramer::SerializeRstStream(
   // which doesn't currently include RST_STREAM payloads. GFE flags have been
   // commented but left in place to simplify future patching.
   // Compute the output buffer size, taking opaque data into account.
-  uint16 expected_length = GetRstStreamMinimumSize();
+  size_t expected_length = GetRstStreamMinimumSize();
   if (protocol_version() > SPDY3) {
     expected_length += rst_stream.description().size();
   }
@@ -2536,6 +2536,9 @@ SpdySerializedFrame* SpdyFramer::SerializeSettings(
   for (SpdySettingsIR::ValueMap::const_iterator it = values->begin();
        it != values->end();
        ++it) {
+    int setting_id =
+        SpdyConstants::SerializeSettingId(protocol_version(), it->first);
+    DCHECK_GE(setting_id, 0);

[Peter Kasting, 2014/11/25 22:51:00]

SerializeSettingId() can return -1 on error, but the code below assumes there's
no error.  I assumed the code was correct and added this DCHECK as documentation
of that assumption.

     if (protocol_version() <= SPDY3) {
       uint8 setting_flags = 0;
       if (it->second.persist_value) {
@@ -2544,14 +2547,11 @@ SpdySerializedFrame* SpdyFramer::SerializeSettings(
       if (it->second.persisted) {
         setting_flags |= SETTINGS_FLAG_PERSISTED;
       }
-      SettingsFlagsAndId flags_and_id(
-          setting_flags,
-          SpdyConstants::SerializeSettingId(protocol_version(), it->first));
+      SettingsFlagsAndId flags_and_id(setting_flags, setting_id);
       uint32 id_and_flags_wire = flags_and_id.GetWireFormat(protocol_version());
       builder.WriteBytes(&id_and_flags_wire, 4);
     } else {
-      builder.WriteUInt16(SpdyConstants::SerializeSettingId(protocol_version(),
-                                                            it->first));
+      builder.WriteUInt16(static_cast<uint16>(setting_id));
     }
     builder.WriteUInt32(it->second.value);
   }
@@ -2580,7 +2580,7 @@ SpdySerializedFrame* SpdyFramer::SerializeGoAway(
     const SpdyGoAwayIR& goaway) const {
 
   // Compute the output buffer size, take opaque data into account.
-  uint16 expected_length = GetGoAwayMinimumSize();
+  size_t expected_length = GetGoAwayMinimumSize();
   if (protocol_version() > SPDY3) {
     expected_length += goaway.description().size();
   }
@@ -2639,9 +2639,9 @@ SpdySerializedFrame* SpdyFramer::SerializeHeaders(
     size += headers.padding_payload_len();
   }
 
-  uint32 priority = headers.priority();
+  SpdyPriority priority = static_cast<SpdyPriority>(headers.priority());
   if (headers.has_priority()) {
-    if (priority > GetLowestPriority()) {
+    if (headers.priority() > GetLowestPriority()) {
       DLOG(DFATAL) << "Priority out-of-bounds.";
       priority = GetLowestPriority();
     }
@@ -2857,10 +2857,10 @@ SpdyFrame* SpdyFramer::SerializeAltSvc(const SpdyAltSvcIR& altsvc) {
   builder.WriteUInt32(altsvc.max_age());
   builder.WriteUInt16(altsvc.port());
   builder.WriteUInt8(0);  // Reserved.
-  builder.WriteUInt8(altsvc.protocol_id().length());
+  builder.WriteUInt8(static_cast<uint8>(altsvc.protocol_id().length()));

[Peter Kasting, 2014/11/25 22:51:00]

I assumed the lengths here and below were guaranteed to fit in a uint8 and there
was no need to check or DCHECK them.  Let me know if this isn't true.

[Bence, 2014/12/01 18:18:38]

SpdyAltSvcIR.host_ and protocol_id_ are std::strings, and set_host() and
set_protocol_id() do not enforce a limit on length.  Could you please add
DCHECKs on length here (or possibly to the set_* methods in spdy_protocol.h)?

[Peter Kasting, 2014/12/02 02:13:55]

Added DCHECKs to the setters, but perhaps these should be CHECKs instead? 
Otherwise it seems like we get a corrupted serialization leading to who knows
what.

That said, AFAICT, those two setters are only ever used by test code.  So
outside of tests, these fields are always empty?  Maybe they should disappear or
something.

[Bence, 2014/12/02 14:58:44]

Hey, I just realized this binary format is obsolete.  Sorry.  This format was
specified in
https://tools.ietf.org/html/draft-ietf-httpbis-alt-svc-04#section-4, but has
been changed in
https://tools.ietf.org/html/draft-ietf-httpbis-alt-svc-05#section-4, and does
not have the length<256 limitation any more.

Could you please delete the DCHECKs from the setters, and add a TODO(bnc) here.
Indeed, and SerializeAltSvc is not currently used in Chrome, nor in the internal
codebase.  We still need the setters for the future when we start emitting
AltSvc frames.  (Or initializing in the constructor, or something.)

[Peter Kasting, 2014/12/03 00:27:08]

Done.

   builder.WriteBytes(altsvc.protocol_id().data(),
                      altsvc.protocol_id().length());
-  builder.WriteUInt8(altsvc.host().length());
+  builder.WriteUInt8(static_cast<uint8>(altsvc.host().length()));
   builder.WriteBytes(altsvc.host().data(), altsvc.host().length());
   builder.WriteBytes(altsvc.origin().data(), altsvc.origin().length());
   DCHECK_LT(GetAltSvcMinimumSize(), builder.length());
@@ -3207,7 +3207,7 @@ void SpdyFramer::SerializeNameValueBlockWithoutCompression(
     const SpdyNameValueBlock& name_value_block) const {
   // Serialize number of headers.
   if (protocol_version() <= SPDY2) {
-    builder->WriteUInt16(name_value_block.size());
+    builder->WriteUInt16(static_cast<uint16>(name_value_block.size()));
   } else {
     builder->WriteUInt32(name_value_block.size());
   }