rtmp: Don't try to do av_malloc(0)

diff mbox

Some received packets can have size 0. The return value from
av_malloc(0) may be NULL, which is ok if the size was 0. On
OS X, however, the returned pointer is non-null but leads to
crashes when trying to free it.
---
 libavformat/rtmppkt.c   |    2 ++
 libavformat/rtmpproto.c |    2 +-
 2 files changed, 3 insertions(+), 1 deletions(-)

Hi,

On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
> @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
>  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
>                           int timestamp, int size)
>  {
> +    if (size) {
>     pkt->data = av_malloc(size);
>     if (!pkt->data)
>         return AVERROR(ENOMEM);
> +    }
>     pkt->data_size  = size;
>     pkt->channel_id = channel_id;
>     pkt->type       = type;

OK.

> @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
>         return AVERROR_EOF;
>
>     for (;;) {
> -        RTMPPacket rpkt;
> +        RTMPPacket rpkt = { 0 };
>         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
>                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
>             if (ret == 0) {

Unrelated? If it fixes something it's OK.

Ronald

On Wed, 25 May 2011, Ronald S. Bultje wrote:

> Hi,
> 
> On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
> > @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
> >  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
> >                           int timestamp, int size)
> >  {
> > +    if (size) {
> >     pkt->data = av_malloc(size);
> >     if (!pkt->data)
> >         return AVERROR(ENOMEM);
> > +    }
> >     pkt->data_size  = size;
> >     pkt->channel_id = channel_id;
> >     pkt->type       = type;
> 
> OK.
> 
> > @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
> >         return AVERROR_EOF;
> >
> >     for (;;) {
> > -        RTMPPacket rpkt;
> > +        RTMPPacket rpkt = { 0 };
> >         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
> >                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
> >             if (ret == 0) {
> 
> Unrelated? If it fixes something it's OK.

No, without this, pkt->data will be left uninitialized for these cases 
where size == 0.

I'll push in a moment then - I assume I don't need to post the reindent 
patch before pushing it.

// Mart

Hi Martin,

On Wed, May 25, 2011 at 2:53 PM, Martin Storsjö <martin@martin.st> wrote:
> On Wed, 25 May 2011, Ronald S. Bultje wrote:
>> On Wed, May 25, 2011 at 1:37 PM, Martin Storsjö <martin@martin.st> wrote:
>> > @@ -233,9 +233,11 @@ int ff_rtmp_packet_write(URLContext *h, RTMPPacket *pkt,
>> >  int ff_rtmp_packet_create(RTMPPacket *pkt, int channel_id, RTMPPacketType type,
>> >                           int timestamp, int size)
>> >  {
>> > +    if (size) {
>> >     pkt->data = av_malloc(size);
>> >     if (!pkt->data)
>> >         return AVERROR(ENOMEM);
>> > +    }
>> >     pkt->data_size  = size;
>> >     pkt->channel_id = channel_id;
>> >     pkt->type       = type;
>>
>> OK.
>>
>> > @@ -683,7 +683,7 @@ static int get_packet(URLContext *s, int for_header)
>> >         return AVERROR_EOF;
>> >
>> >     for (;;) {
>> > -        RTMPPacket rpkt;
>> > +        RTMPPacket rpkt = { 0 };
>> >         if ((ret = ff_rtmp_packet_read(rt->stream, &rpkt,
>> >                                        rt->chunk_size, rt->prev_pkt[0])) <= 0) {
>> >             if (ret == 0) {
>>
>> Unrelated? If it fixes something it's OK.
>
> No, without this, pkt->data will be left uninitialized for these cases
> where size == 0.
>
> I'll push in a moment then - I assume I don't need to post the reindent
> patch before pushing it.

Nope, that's fine.

Ronald