[FFmpeg-devel] [PATCH] rmdec: avoid leaving st unitialized

[Ronald S. Bultje]

Hi,

On Sun, Jun 6, 2010 at 2:45 PM, Reimar D?ffinger
<Reimar.Doeffinger at gmx.de> wrote:
> On Sun, Jun 06, 2010 at 01:03:20PM -0400, Ronald S. Bultje wrote:
>> On Sun, Jun 6, 2010 at 6:57 AM, Reimar D?ffinger
>> <Reimar.Doeffinger at gmx.de> wrote:
>> > this fixes a warning with my gcc version, and I think it even fixes a bug:
>> > Index: rmdec.c
>> > ===================================================================
>> > --- rmdec.c ? ? (revision 23501)
>> > +++ rmdec.c ? ? (working copy)
>> > @@ -843,7 +844,7 @@
>> > ? ? ? ? ? ? ? ? pos = url_ftell(s->pb);
>> > ? ? ? ? ? ? } else {
>> > ? ? ? ? ? ? ? ? len=sync(s, &timestamp, &flags, &i, &pos);
>> > - ? ? ? ? ? ? ? ?if (len > 0)
>> > + ? ? ? ? ? ? ? ?if (len >= 0)
>> > ? ? ? ? ? ? ? ? ? ? st = s->streams[i];
>> > ? ? ? ? ? ? }
>> >
>> >
>> > The next line below this reads:
>> > ? ? ? ? ? ?if(len<0 || url_feof(s->pb))
>> > ? ? ? ? ? ? ? ?return AVERROR(EIO);
>> > so >= 0 fits with this condition, and other places treat a 0 value
>> > from "sync" as ok.
>>
>> sync() can not return zero (I've looked at this a couple of times in
>> the past because the gcc warning annoyed me at some point). So the
>> patch silences a warning but in reality, it doesn't do anything.
>
> ok, no bug, but that still leaves a compiler warning and that there's
> no point to treat 0 once this way once the other way...
> And if you "looked at this a couple of times" there's also obviously
> the point that it obviously wastes developer time the way it is now.

It's like tossing a coin, shall we treat 0 as error or as success?
:-). I'd say error, but please then document that in reality this will
never happen and that it's just there to silence a compiler warning
(or so). OK with you Kostya?

Ronald