From 19f5694277747c514f22256d203f509bb6c1cd08 Mon Sep 17 00:00:00 2001 From: Tom Finegan Date: Mon, 31 Aug 2015 13:59:35 -0700 Subject: [PATCH] mkvparser: Cluster::Load clean up. - Remove "// weird"'s. - Remove needless extra scoping. - Asserts to error checks. Change-Id: I5c4d1bbc59d9debe95e1e35e63ff0679335048ff --- mkvparser.cpp | 142 +++++++++++++++++++++++--------------------------- 1 file changed, 66 insertions(+), 76 deletions(-) diff --git a/mkvparser.cpp b/mkvparser.cpp index 9a08f93..337540c 100644 --- a/mkvparser.cpp +++ b/mkvparser.cpp @@ -5650,98 +5650,88 @@ const Track* Tracks::GetTrackByIndex(unsigned long idx) const { } long Cluster::Load(long long& pos, long& len) const { - assert(m_pSegment); - assert(m_pos >= m_element_start); + if (m_pSegment == NULL) + return E_PARSE_FAILED; if (m_timecode >= 0) // at least partially loaded return 0; - assert(m_pos == m_element_start); - assert(m_element_size < 0); + if (m_pos != m_element_start || m_element_size >= 0) + return E_PARSE_FAILED; IMkvReader* const pReader = m_pSegment->m_pReader; - long long total, avail; - const int status = pReader->Length(&total, &avail); if (status < 0) // error return status; - assert((total < 0) || (avail <= total)); - assert((total < 0) || (m_pos <= total)); // TODO: verify this + if (total >= 0 && (avail > total || m_pos > total)) + return E_FILE_FORMAT_INVALID; pos = m_pos; long long cluster_size = -1; - { - if ((pos + 1) > avail) { - len = 1; - return E_BUFFER_NOT_FULL; - } - - long long result = GetUIntLength(pReader, pos, len); - - if (result < 0) // error or underflow - return static_cast(result); - - if (result > 0) // underflow (weird) - return E_BUFFER_NOT_FULL; - - // if ((pos + len) > segment_stop) - // return E_FILE_FORMAT_INVALID; - - if ((pos + len) > avail) - return E_BUFFER_NOT_FULL; - - const long long id_ = ReadUInt(pReader, pos, len); - - if (id_ < 0) // error - return static_cast(id_); - - if (id_ != 0x0F43B675) // Cluster ID - return E_FILE_FORMAT_INVALID; - - pos += len; // consume id - - // read cluster size - - if ((pos + 1) > avail) { - len = 1; - return E_BUFFER_NOT_FULL; - } - - result = GetUIntLength(pReader, pos, len); - - if (result < 0) // error - return static_cast(result); - - if (result > 0) // weird - return E_BUFFER_NOT_FULL; - - // if ((pos + len) > segment_stop) - // return E_FILE_FORMAT_INVALID; - - if ((pos + len) > avail) - return E_BUFFER_NOT_FULL; - - const long long size = ReadUInt(pReader, pos, len); - - if (size < 0) // error - return static_cast(cluster_size); - - if (size == 0) - return E_FILE_FORMAT_INVALID; // TODO: verify this - - pos += len; // consume length of size of element - - const long long unknown_size = (1LL << (7 * len)) - 1; - - if (size != unknown_size) - cluster_size = size; + if ((pos + 1) > avail) { + len = 1; + return E_BUFFER_NOT_FULL; } + long long result = GetUIntLength(pReader, pos, len); + + if (result < 0) // error or underflow + return static_cast(result); + + if (result > 0) + return E_BUFFER_NOT_FULL; + + if ((pos + len) > avail) + return E_BUFFER_NOT_FULL; + + const long long id_ = ReadUInt(pReader, pos, len); + + if (id_ < 0) // error + return static_cast(id_); + + if (id_ != 0x0F43B675) // Cluster ID + return E_FILE_FORMAT_INVALID; + + pos += len; // consume id + + // read cluster size + + if ((pos + 1) > avail) { + len = 1; + return E_BUFFER_NOT_FULL; + } + + result = GetUIntLength(pReader, pos, len); + + if (result < 0) // error + return static_cast(result); + + if (result > 0) + return E_BUFFER_NOT_FULL; + + if ((pos + len) > avail) + return E_BUFFER_NOT_FULL; + + const long long size = ReadUInt(pReader, pos, len); + + if (size < 0) // error + return static_cast(cluster_size); + + if (size == 0) + return E_FILE_FORMAT_INVALID; + + pos += len; // consume length of size of element + + const long long unknown_size = (1LL << (7 * len)) - 1; + + if (size != unknown_size) + cluster_size = size; + // pos points to start of payload long long timecode = -1; long long new_pos = -1; @@ -5765,7 +5755,7 @@ long Cluster::Load(long long& pos, long& len) const { if (result < 0) // error return static_cast(result); - if (result > 0) // weird + if (result > 0) return E_BUFFER_NOT_FULL; if ((cluster_stop >= 0) && ((pos + len) > cluster_stop)) @@ -5806,7 +5796,7 @@ long Cluster::Load(long long& pos, long& len) const { if (result < 0) // error return static_cast(result); - if (result > 0) // weird + if (result > 0) return E_BUFFER_NOT_FULL; if ((cluster_stop >= 0) && ((pos + len) > cluster_stop)) @@ -5832,7 +5822,7 @@ long Cluster::Load(long long& pos, long& len) const { // pos now points to start of payload - if (size == 0) // weird + if (size == 0) continue; if ((cluster_stop >= 0) && ((pos + size) > cluster_stop))