[FFmpeg-devel] [PATCH] lavu/common: make GET_UTF8 less broken

[Reimar Döffinger]

On Wed, Oct 02, 2013 at 08:22:05PM +0200, Stefano Sabatini wrote:
> On date Wednesday 2013-10-02 20:17:30 +0200, Stefano Sabatini encoded:
> > Avoid to process data and push the pointer forward in case as soon as an
> > error is encountered.
> > 
> > For example if the error is an assignment, this will allow to exit the
> > while block as soon as possible.
> > ---
> >  libavutil/common.h | 16 +++++++++++-----
> >  1 file changed, 11 insertions(+), 5 deletions(-)
> > 
> > diff --git a/libavutil/common.h b/libavutil/common.h
> > index 9f0f998..58f7e42 100644
> > --- a/libavutil/common.h
> > +++ b/libavutil/common.h
> > @@ -300,14 +300,20 @@ static av_always_inline av_const int av_popcount64_c(uint64_t x)
> >      val= GET_BYTE;\
> >      {\
> >          uint32_t top = (val & 128) >> 1;\
> > -        if ((val & 0xc0) == 0x80 || val >= 0xFE)\
> > +        int err = 0;\
> > +        if ((val & 0xc0) == 0x80 || val >= 0xFE) {\
> > +            err = 1;\
> >              ERROR\
> > -        while (val & top) {\
> > +        }\
> > +        while (!err && (val & top)) {\
> >              int tmp= GET_BYTE - 128;\
> > -            if(tmp>>6)\
> > +            if (tmp>>6) {\
> > +                err = 1;\
> >                  ERROR\
> > -            val= (val<<6) + tmp;\
> > -            top <<= 5;\
> > +            } else {\
> > +                val x = (val<<6) + tmp;\
> > +                top <<= 5;\
> > +            }\
> >          }\
> >          val &= (top << 1) - 1;\
> >      }
> 
> Other problems: in the while loop, the characters should probably not
> consumed, and the pointer restored to the second character.
> 
> In case ERROR is a struck control operation (e.g. "continue;" or
> "break;") it will affect the while loop context, and not the external
> user loop as intended.
> 
> In conclusion we should probably replace this with an inline function
> or something faintly robust.

I think this is supposed to be a high-performance primitive,
and ERROR is not supposed to be anything other than a return
or goto.
"Bloating" it even more IMHO defeats the purpose.
I'd suggest providing a (possibly inline) function that
is less tricky to use, and warning that this function is
not intended to easily allow complex error handling.