[FFmpeg-devel] [PATCH] urlconcat protocol

[Michael Niedermayer]

On Mon, Feb 01, 2010 at 10:53:17PM +0100, Michele Orr? wrote:
> 2010/1/31 Michael Niedermayer <michaelni at gmx.at>
> 
> > On Sat, Jan 30, 2010 at 11:35:56PM +0100, Michele Orr? wrote:
> > [...]
> > > +static int urlconcat_open(URLContext *h, const char *uri, int flags)
> > > +{
> > > +    char *upath = NULL;
> > > +    int err = 0;
> > > +    int64_t size;
> > > +    size_t  len, i;
> > > +    URLContext *uc;
> > > +    struct urlconcat_data *udata;
> > > +    struct urlconcat_nodes *unodes;
> > > +
> > > +    av_strstart(uri, "cat:", &uri);
> > > +
> > > +    /* creating udata */
> > > +    if (!(udata = av_mallocz(sizeof(*udata))))
> > > +        return AVERROR(ENOMEM);
> > > +    h->priv_data = udata;
> > > +    /* creating udata->urls */
> > > +    for (i=0, len = 1; uri[i]; i++)  /* cat:[url]|[url] -> urls = sep+1
> > */
> > > +        if (uri[i] == *AV_CAT_SEPARATOR) len++;
> > > +    if (!(unodes = av_malloc(sizeof(*unodes) * len))) {
> >
> > integer overflow that could lead to a too small buffer to be allocated
> >
> 
> Oh, that's right. Fixed with a simple check, but.. If you don't like this
> solution I can recycle the old linked list approach.

no i actually dont think that can overflow rather the multiplication can


> 
> 
> >
> > > +        av_free(udata);
> > > +        h->priv_data = NULL;
> > > +        return AVERROR(ENOMEM);
> > > +    } else
> > > +        udata->urls = unodes;
> > > +
> > > +    /* handle input */
> > > +    if (!*uri) err = AVERROR(ENOENT);
> > > +    for (i = 0; *uri; i++) {
> > > +        /* parsing uri */
> > > +        len = strcspn(uri, AV_CAT_SEPARATOR);
> >
> > > +        upath = av_realloc(upath, len);
> > > +        if (!upath) {
> > > +            err = AVERROR(ENOMEM);
> > > +            break;
> > > +        }
> >
> > memleak, av_realloc() does not free its input buffer if it returns NULL
> >
> Added a check here, too

good
a few empty lines at strategic points could also help readability a little
but thats not important

> .
> 

> >
> >
> > > +        av_strlcpy(upath, uri, len+1);
> >
> > +1 ?
> >
> yep.    ._.

and how large is the destination array?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Democracy is the form of government in which you can choose your dictator
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20100202/0c92f06c/attachment.pgp>