[FFmpeg-devel] [PATCH] make sure initialization happens even after goto fires

[Måns Rullgård]

Reimar D?ffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de> writes:

> On Thu, Jul 17, 2008 at 11:12:04AM -0700, Erik Hovland wrote:
>> On Thu, Jul 17, 2008 at 07:03:43PM +0200, Michael Niedermayer wrote:
>> > On Thu, Jul 17, 2008 at 09:08:40AM -0700, Erik Hovland wrote:
>> > > The goto in ff_h264_find_frame_end() is inside an else clause. The
>> > > initialization for v happens in the if clause elsewhere. That means that
>> > > it is possible for the goto to fire without the initialization of v
>> > > happening. Normally this isn't a problem. But gcc recognizes this and
>> > > throws an internal parse warning. It then has to go through a special
>> > > code branch to take care of this. If the initialization of v always
>> > > happens before either clause this does not happen. The attached patch
>> > > does just that.
>> > 
>> > Is there any bug in our code?
>> 
>> Well, that is an interesting question. It would be a bug if the goto
>> fired without v being initialized. Which is possible, but highly
>> unlikely.
>
> Why, v is not used so why should it be wrong if it is not initialized?
> Still, I do think the code would look nicer if it was done like in
> attached patch, though the remaining code is sufficient to make the
> average programmer's head explode on first sight anyway ;-)
>
> Greetings,
> Reimar D?ffinger
>
> Index: libavcodec/h264_parser.c
> ===================================================================
> --- libavcodec/h264_parser.c	(revision 14263)
> +++ libavcodec/h264_parser.c	(working copy)
> @@ -59,10 +59,7 @@
>              if(v==7 || v==8 || v==9){
>                  if(pc->frame_start_found){
>                      i++;
> -found:
> -                    pc->state=7;
> -                    pc->frame_start_found= 0;
> -                    return i-(state&5);
> +                    goto found;
>                  }
>              }else if(v==1 || v==2 || v==5){
>                  if(pc->frame_start_found){
> @@ -80,6 +77,11 @@
>      }
>      pc->state= state;
>      return END_NOT_FOUND;
> +
> +found:
> +    pc->state=7;
> +    pc->frame_start_found= 0;
> +    return i-(state&5);
>  }

The question is, is this as fast?  I agree it's nicer structurally.

-- 
M?ns Rullg?rd
mans at mansr.com