Currently read_data() of mp4.c is an atrocious mess. The ->read()
callback is defined to return uint32_t, but the return value is
stored in a signed 32 bit integer. Moreover, read_data() contains a
dead store, it handles neither short nor interrupted reads correctly,
and it moves the file position backwards on errors.
While this is easy to fix, a more intricate problem is that most
callers of read_data(), including all read_intX() helpers, ignore the
return value of read_data() and return uninitialized stack contents in
the error case. This is kind of dealt with by the ->read_error member
of struct mp4, but this not more than a kludge, which, according to
the comments, was applied after several CVEs had been filed against
the library.
Let's DTRT here, even though it adds a fair amount of new code:
Check the return value of each read operation and fail early on errors.
We have to distinguish three cases: error, EOF, and success, encoded
as return values -1, 0 and 1, respectively. This commit converts most
functions which read from an mp4 file to this convention. More work
is required as return values are not checked everywhere yet. This was
left for subsequent commits to keep the already large patch within
reasonable size.
Since we don't rely on ->read_error of struct mp4 any more, it can
be removed.