On Wed, 18 Mar 2020 16:05:40 +0100
"Andreas Rheinhardt" <andreas.rheinha...@gmail.com> wrote:
> > +
> > +typedef struct PPBnkCtx {
> > + int track_count;
> > + PPBnkCtxTrack *tracks;
> > + uint32_t i;
>
> How about using a more descriptive name for this like current_track?
>
Done.
> > +
> > + ctx->track_count = hdr.track_count;
> > + if ((ret = av_reallocp_array(&ctx->tracks, hdr.track_count,
> > sizeof(PPBnkCtxTrack))))
>
> read_close is not called if read_header fails and therefore this array
> leaks if any of the following things that can fail fail.
> (It would probably make sense to call it automatically, but this will
> require changes in some demuxers.)
>
Fixed, I assumed it was called irregardless of failure.
I wonder how much work it would be to port all of the demuxers to
handle this...
> > + ctx->bytes_read = 0;
> > + ctx->i += 1;
> > + trk += 1;
>
> Why don't you simply use an increment ++?
>
Stylistic reasons. Is moot anyway as I had to change that code to fix a
different bug.
> > +
> > + if ((ret = avio_seek(s->pb, trk->data_offset, SEEK_SET)) <
> > 0)
>
> avio_seek returns int64_t, yet ret is only an int. This will cause
> problems when you seek to positions from 2GB to 4GB.
>
Fixed.
> > + return ret;
> > + else if (ret != trk->data_offset)
> > + return AVERROR(EIO);
>
> I wonder whether the seek should be attempted before you set
> bytes_read and i. If the seek fails and the caller retries to read a
> packet, it will (try to) read the wrong data.
>
Fixed. This lead me to another issue where it overreads trk before
checking for EOF (also fixed).
> > +static int pp_bnk_read_close(AVFormatContext *s)
> > +{
> > + PPBnkCtx *ctx = s->priv_data;
> > +
> > + if (ctx->tracks)
> > + av_freep(&ctx->tracks);
>
> The check here is unnecessary as av_freep() already checks for this;
> and moreover, read_close is only called if read_header succeeded, so
> ctx->tracks is always != NULL.
Fixed.
Zane
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
