Cleanup media BitReader ReadBits() calls

Initialize temporary values, check return values.
Small tweaks to solution proposed by adtolbar@microsoft.com.

Bug: 929962
Change-Id: Iaa7da7534174882d040ec7e4c353ba5cd0da5735
Reviewed-on: https://chromium-review.googlesource.com/c/1481085
Commit-Queue: Chrome Cunningham <chcunningham@chromium.org>
Reviewed-by: Dan Sanders <sandersd@chromium.org>
Cr-Commit-Position: refs/heads/master@{#634889}

media/base/bit_reader_core.cc

@@ -113,6 +113,7 @@ bool BitReaderCore::ReadBitsInternal(int num_bits, uint64_t* out) {
     // empty the current bit register for that purpose.
     nbits_ = 0;
     reg_ = 0;
+    *out = 0;
     return false;
   }
 

media/base/bit_reader_core.h

@@ -56,9 +56,10 @@ class MEDIA_EXPORT BitReaderCore {
   // integer type.
   template<typename T> bool ReadBits(int num_bits, T* out) {
     DCHECK_LE(num_bits, static_cast<int>(sizeof(T) * 8));
-    uint64_t temp;
+    uint64_t temp = 0;
     bool ret = ReadBitsInternal(num_bits, &temp);
-    *out = static_cast<T>(temp);
+    if (ret)
+      *out = static_cast<T>(temp);
     return ret;
   }
 

media/base/container_names.cc

@@ -73,11 +73,17 @@ static bool StartsWith(const uint8_t* buffer,
 }
 
 // Helper function to read up to 64 bits from a bit stream.
+// TODO(chcunningham): Delete this helper and replace with direct calls to
+// reader that handle read failure. As-is, we hide failure because returning 0
+// is valid for both a successful and failed read.
 static uint64_t ReadBits(BitReader* reader, int num_bits) {
   DCHECK_GE(reader->bits_available(), num_bits);
   DCHECK((num_bits > 0) && (num_bits <= 64));
-  uint64_t value;
-  reader->ReadBits(num_bits, &value);
+  uint64_t value = 0;
+
+  if (!reader->ReadBits(num_bits, &value))
+    return 0;
+
   return value;
 }
 
@@ -304,7 +310,9 @@ static bool CheckDts(const uint8_t* buffer, int buffer_size) {
     reader.SkipBits(6);
 
     // Verify core audio sampling frequency is an allowed value.
-    RCHECK(kSamplingFrequencyValid[ReadBits(&reader, 4)]);
+    size_t sampling_freq_index = ReadBits(&reader, 4);
+    RCHECK(sampling_freq_index < base::size(kSamplingFrequencyValid));
+    RCHECK(kSamplingFrequencyValid[sampling_freq_index]);
 
     // Verify transmission bit rate is valid.
     RCHECK(ReadBits(&reader, 5) <= 25);
@@ -316,7 +324,9 @@ static bool CheckDts(const uint8_t* buffer, int buffer_size) {
     reader.SkipBits(1 + 1 + 1 + 1);
 
     // Verify extension audio descriptor flag is an allowed value.
-    RCHECK(kExtAudioIdValid[ReadBits(&reader, 3)]);
+    size_t audio_id_index = ReadBits(&reader, 3);
+    RCHECK(audio_id_index < base::size(kExtAudioIdValid));
+    RCHECK(kExtAudioIdValid[audio_id_index]);
 
     // Skip extended coding flag and audio sync word insertion flag.
     reader.SkipBits(1 + 1);