[FFmpeg-devel] [PATCH 5/9] lavf/oggdec: rework allocations in ogg_new_streams().

[Clément Bœsch]

On Sat, Sep 15, 2012 at 01:30:09AM +0200, Reimar Döffinger wrote:
> On Sat, Sep 15, 2012 at 01:20:44AM +0200, Clément Bœsch wrote:
> > ---
> >  libavformat/oggdec.c | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
> > index 05aeddd..451e392 100644
> > --- a/libavformat/oggdec.c
> > +++ b/libavformat/oggdec.c
> > @@ -169,14 +169,18 @@ static int ogg_new_stream(AVFormatContext *s, uint32_t serial, int new_avstream)
> >      AVStream *st;
> >      struct ogg_stream *os;
> >  
> > -    ogg->streams = av_realloc (ogg->streams,
> > -                               ogg->nstreams * sizeof (*ogg->streams));
> > +    ogg->streams = av_realloc_f(ogg->streams, ogg->nstreams,
> > +                                sizeof(*ogg->streams));
> > +    if (!ogg->streams)
> > +        return AVERROR(ENOMEM);
> >      memset (ogg->streams + idx, 0, sizeof (*ogg->streams));
> >      os = ogg->streams + idx;
> >      os->serial = serial;
> >      os->bufsize = DECODER_BUFFER_SIZE;
> >      os->buf = av_malloc(os->bufsize + FF_INPUT_BUFFER_PADDING_SIZE);
> >      os->header = -1;
> > +    if (!os->buf)
> > +        return AVERROR(ENOMEM);
> 
> I don't think nstreams should be incremented in the failure case?
> Also the realloc is not fully correct, it will still leak memory on
> failure (sorry if some later patch addresses this, just going through in
> order)

You're totally right, and no worry, no later patch addresses this problem.
The new attached patch should be better.

-- 
Clément B.
-------------- next part --------------
From 3208fa20e4674f1c383aa62eaefa388d8a10d26b Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Cl=C3=A9ment=20B=C5=93sch?= <ubitux at gmail.com>
Date: Fri, 14 Sep 2012 23:51:30 +0200
Subject: [PATCH 5/9] lavf/oggdec: rework allocations in ogg_new_streams().

---
 libavformat/oggdec.c | 12 +++++++++---
 1 file changed, 9 insertions(+), 3 deletions(-)

diff --git a/libavformat/oggdec.c b/libavformat/oggdec.c
index 05aeddd..fbc35dd 100644
--- a/libavformat/oggdec.c
+++ b/libavformat/oggdec.c
@@ -165,18 +165,24 @@ static int ogg_new_stream(AVFormatContext *s, uint32_t serial, int new_avstream)
 {
 
     struct ogg *ogg = s->priv_data;
-    int idx = ogg->nstreams++;
+    int idx = ogg->nstreams;
     AVStream *st;
     struct ogg_stream *os;
+    size_t size;
 
-    ogg->streams = av_realloc (ogg->streams,
-                               ogg->nstreams * sizeof (*ogg->streams));
+    if (av_size_mult(ogg->nstreams + 1, sizeof(*ogg->streams), &size) < 0 ||
+        !(os = av_realloc(ogg->streams, size)))
+        return AVERROR(ENOMEM);
+    ogg->streams = os;
+    ogg->nstreams++;
     memset (ogg->streams + idx, 0, sizeof (*ogg->streams));
     os = ogg->streams + idx;
     os->serial = serial;
     os->bufsize = DECODER_BUFFER_SIZE;
     os->buf = av_malloc(os->bufsize + FF_INPUT_BUFFER_PADDING_SIZE);
     os->header = -1;
+    if (!os->buf)
+        return AVERROR(ENOMEM);
 
     if (new_avstream) {
         st = avformat_new_stream(s, NULL);
-- 
1.7.12

-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 490 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120915/bcb25e45/attachment.asc>