[FFmpeg-devel] avidec.c entering an infinite loop with HTTP URLs + Patch

[Michael Niedermayer]

Hi

On Wed, Jul 04, 2007 at 07:34:38PM -0400, Ronen Mizrahi wrote:
> Hi,
> 
> I have encountered a situation where url_fseek() fails, but its return 
> value is not checked and hence the surrounding code enters an infinite loop.
> The relevant code in in avidec.c and since I am running on Windows with 
> MSVC I do not have a GDB dump to offer. I did however modify avidec.c 
> (see patch attached) such that return values are checked and the problem 
> was eliminated. I would be grateful if someone can apply the patch 
> and/or comment to it. The patch wad made against the current SVN head 
> (revision 8742 for avidec.c).
> 
> Thank you,
> 
> Ronen Mizrahi

> --- avidec.c	Wed Jul 04 19:21:28 2007
> +++ \avidec.c	Wed Jul 04 19:09:02 2007
> @@ -163,14 +163,11 @@
>              duration = get_le32(pb);
>              pos = url_ftell(pb);
>  
> -            if (url_fseek(pb, offset+8, SEEK_SET) < 0)
> -                return -1;
> -            if (read_braindead_odml_indx(s, frame_num) < 0)
> -                return -1;
> +            url_fseek(pb, offset+8, SEEK_SET);
> +            read_braindead_odml_indx(s, frame_num);
>              frame_num += duration;
>  
> -            if (url_fseek(pb, pos, SEEK_SET) < 0)
> -                return -1;
> +            url_fseek(pb, pos, SEEK_SET);
>          }
>      }
>      avi->index_loaded=1;
> @@ -208,8 +205,7 @@
>      offset_t i = url_ftell(pb);
>      size += (size & 1);
>      get_strz(pb, buf, maxlen);
> -    if (url_fseek(pb, i+size, SEEK_SET) < 0)
> -        return -1;
> +    url_fseek(pb, i+size, SEEK_SET);
>      return 0;
>  }
>  

the patch looks reversed, also it seems you check everything and return -1
this is overkill also it would require me to do your work checking that
this actually is correct and that return -1 from these places does not
introduce bugs
so please only change the code which is related to the bug

if you think every url_fseek() should have its return checked then this 
should be a seperate disscussion, thread and patch

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

Everything should be made as simple as possible, but not simpler.
-- Albert Einstein
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070705/ee088689/attachment.pgp>