On Mon, Dec 09, 2013 at 07:24:49PM -0500, Sean McGovern wrote:
> --- a/libavformat/oggdec.c
> +++ b/libavformat/oggdec.c
> @@ -72,12 +76,17 @@ static int ogg_save(AVFormatContext *s)
> for (i = 0; i < ogg->nstreams; i++) {
> struct ogg_stream *os = ogg->streams + i;
> os->buf = av_mallocz(os->bufsize + FF_INPUT_BUFFER_PADDING_SIZE);
> + if (!os->buf) {
> + ret = AVERROR(ENOMEM);
> + av_freep(ost);
> + break;
> + }
> memcpy(os->buf, ost->streams[i].buf, os->bufpos);
> }
>
> ogg->state = ost;
>
> - return 0;
> + return ret;
> }
You free ost, then you assign ost to ogg->state; I'm scared.
> @@ -173,6 +183,12 @@ static int ogg_new_stream(AVFormatContext *s, uint32_t
> serial, int new_avstream)
> os->header = -1;
> os->start_granule = OGG_NOGRANULE_VALUE;
>
> + if (!os->buf) {
> + ret = AVERROR(ENOMEM);
> + av_freep(os);
> + return ret;
> + }
The indirection through the ret variable is silly.
> if (new_avstream) {
> st = avformat_new_stream(s, NULL);
> if (!st)
Here after st is checked you need to free the above allocations.
> @@ -276,8 +295,11 @@ static int ogg_read_page(AVFormatContext *s, int *str)
> os = ogg->streams + idx;
> os->page_pos = avio_tell(bc) - 27;
>
> - if (os->psize > 0)
> - ogg_new_buf(ogg, idx);
> + if (os->psize > 0) {
> + ret = ogg_new_buf(ogg, idx);
> + if (ret < 0)
> + return ret;
> + }
>
> ret = avio_read(bc, os->segments, nsegs);
> if (ret < nsegs)
I'm not quite sure yet what needs to be done in case the av_malloc
below fails.
> @@ -173,6 +183,12 @@ static int ogg_new_stream(AVFormatContext *s, uint32_t
> serial, int new_avstream)
> os->header = -1;
> os->start_granule = OGG_NOGRANULE_VALUE;
>
> + if (!os->buf) {
> + ret = AVERROR(ENOMEM);
> + av_freep(os);
> + return ret;
> + }
The indirection through the ret variable is silly.
> @@ -276,8 +295,11 @@ static int ogg_read_page(AVFormatContext *s, int *str)
> os = ogg->streams + idx;
> os->page_pos = avio_tell(bc) - 27;
>
> - if (os->psize > 0)
> - ogg_new_buf(ogg, idx);
> + if (os->psize > 0) {
> + ret = ogg_new_buf(ogg, idx);
> + if (ret < 0)
> + return ret;
> + }
There are memory allocations above and below this. I have to investigate
what needs to be done when I'm more awake.
Diego
_______________________________________________
libav-devel mailing list
[email protected]
https://lists.libav.org/mailman/listinfo/libav-devel
