From 841a9b5fd90d82a7260bb8092bc22b716d759bda Mon Sep 17 00:00:00 2001
From: Tom Finegan <tomfinegan@google.com>
Date: Fri, 28 Aug 2015 14:53:47 -0700
Subject: [PATCH] mkvparser: Segment assert clean up.

- Remove asserts from ~Segment, CreateInstance, and DoLoadCluster.
- Adds new return constant (E_PARSE_FAILED == -1); most places in
  mkvparser that return -1 are the result of an internal inconsistency
  and/or misusing the API. In those cases E_FILE_FORMAT_INVALID is not
  appropriate as a return value because it's misleading to libwebm users.

Change-Id: I0d46e831b3475d69432b8745066de3329419fa11
---
 mkvparser.cpp | 31 +++++++++++++++++--------------
 mkvparser.hpp |  1 +
 2 files changed, 18 insertions(+), 14 deletions(-)

diff --git a/mkvparser.cpp b/mkvparser.cpp
index 711a7e7..467517c 100644
--- a/mkvparser.cpp
+++ b/mkvparser.cpp
@@ -632,8 +632,6 @@ Segment::~Segment() {
 
   while (i != j) {
     Cluster* const p = *i++;
-    assert(p);
-
     delete p;
   }
 
@@ -649,8 +647,8 @@ Segment::~Segment() {
 
 long long Segment::CreateInstance(IMkvReader* pReader, long long pos,
                                   Segment*& pSegment) {
-  assert(pReader);
-  assert(pos >= 0);
+  if (pReader == NULL || pos < 0)
+    return E_PARSE_FAILED;
 
   pSegment = NULL;
 
@@ -993,7 +991,8 @@ long Segment::DoLoadCluster(long long& pos, long& len) {
   if (status < 0)  // error
     return status;
 
-  assert((total < 0) || (avail <= total));
+  if (total >= 0 && avail > total)
+    return E_FILE_FORMAT_INVALID;
 
   const long long segment_stop = (m_size < 0) ? -1 : m_start + m_size;
 
@@ -1114,7 +1113,10 @@ long Segment::DoLoadCluster(long long& pos, long& len) {
     break;
   }
 
-  assert(cluster_off >= 0);  // have cluster
+  if (cluster_off < 0) {
+    // No cluster, die.
+    return E_FILE_FORMAT_INVALID;
+  }
 
   long long pos_;
   long len_;
@@ -1160,14 +1162,16 @@ long Segment::DoLoadCluster(long long& pos, long& len) {
   const long idx = m_clusterCount;
 
   if (m_clusterPreloadCount > 0) {
-    assert(idx < m_clusterSize);
+    if (idx >= m_clusterSize)
+      return E_FILE_FORMAT_INVALID;
 
     Cluster* const pCluster = m_clusters[idx];
-    assert(pCluster);
-    assert(pCluster->m_index < 0);
+    if (pCluster == NULL || pCluster->m_index >= 0)
+      return E_FILE_FORMAT_INVALID;
 
     const long long off = pCluster->GetPosition();
-    assert(off >= 0);
+    if (off < 0)
+      return E_FILE_FORMAT_INVALID;
 
     if (off == cluster_off) {  // preloaded already
       if (status == 0)  // no entries found
@@ -1224,15 +1228,14 @@ long Segment::DoLoadCluster(long long& pos, long& len) {
     delete pCluster;
     return -1;
   }
-  assert(m_clusters);
-  assert(idx < m_clusterSize);
-  assert(m_clusters[idx] == pCluster);
 
   if (cluster_size >= 0) {
     pos += cluster_size;
 
     m_pos = pos;
-    assert((segment_stop < 0) || (m_pos <= segment_stop));
+
+    if (segment_stop > 0 && m_pos > segment_stop)
+      return E_FILE_FORMAT_INVALID;
 
     return 0;
   }
diff --git a/mkvparser.hpp b/mkvparser.hpp
index d69fae2..75ef69d 100644
--- a/mkvparser.hpp
+++ b/mkvparser.hpp
@@ -15,6 +15,7 @@
 
 namespace mkvparser {
 
+const int E_PARSE_FAILED = -1;
 const int E_FILE_FORMAT_INVALID = -2;
 const int E_BUFFER_NOT_FULL = -3;