[MPlayer-dev-eng] [PATCH] Libass BOM fix

[Ivan Kalvachev]

2007/10/28, Ulion <ulion2002 at gmail.com>:
> 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > 2007/10/28, Ulion <ulion2002 at gmail.com>:
> > > > 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > > > 2007/10/28, Ulion <ulion2002 at gmail.com>:
> > > > > > 2007/10/28, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > > > > > 2007/10/27, Ulion <ulion2002 at gmail.com>:
> > > > > > > > 2007/10/27, Ivan Kalvachev <ikalvachev at gmail.com>:
> > > > > > > > > 2007/10/27, Ulion <ulion2002 at gmail.com>:
> > > > > > > > > > 2007/10/27, Reimar Döffinger <Reimar.Doeffinger at stud.uni-karlsruhe.de>:
> > > > > > > > > > > Hello,
> > > > > > > > > > > On Sat, Oct 27, 2007 at 10:01:29PM +0800, Ulion wrote:
> > > > > > > > > > > > A memory leak bug found in sub_recode, my fix is here:
> > > > > > > > > > >
> > > > > > > > > > > Better don't send patches for new issues as replies.
> > > > > > > > > > > Also if you send them inline take care your mailer doesn't wrap lines.
> > > > > > > > > >
> > > > > > > > > > Sorry for that. I sent it inline to be noticed directly, it's just a
> > > > > > > > > > notifaction.
> > > > > > > > > >
> > > > > > > > > > > And attached patch seems simpler (and would allow to remove the later
> > > > > > > > > > > #ifdef HAVE_ENCA).
> > > > > > > > > > > Note that it is untested though.
> > > > > > > > > >
> > > > > > > > > > Your one is simper for resolve this little leak.
> > > > > > > > > > But as ivan suggested, maybe we should get rid of all strdup stuff for
> > > > > > > > > > enca, I think we should wait for his patch.
> > > > > > > > >
> > > > > > > > > Here it is.
> > > > > > > > > Have in mind there is something fishy with it. It compiled and run
> > > > > > > > > from the first time.
> > > > > > > > > According to Murphy's law, it means that the concept is flowed. ;)
> > > > > > > > >
> > > > > > > >
> > > > > > > > Variable tmp may removed since it's not used any more.
> > > > > > >
> > > > > > > It is used.
> > > > > > > The ENCS_CS_UNKNOWN is valid to enca_charset_name() and it won't
> > > > > > > return NULL. (at least this is how I understand
> > > > > > > http://trific.ath.cx/software/enca/libenca/libenca-Charsets-and-Surfaces/#enca-charset-name
> > > > > > >  )
> > > > > > >
> > > > > > > Of course I can move the check before the enca_charset_name(), but
> > > > > > > then I'd have to check for NULL again later, probably at the end
> > > > > > > (using the else).
> > > > > > > This clean up is not really related to the strdup removal, so I prefer
> > > > > > > to do it in 2 steps.
> > > > > > > First the patch I posted earlier and then the clean-up variable patch
> > > > > > > I attach now.
> > > > > >
> > > > > > About the step 2, you check ENCS_CS_UNKNOWN first, and after
> > > > > > enca_charset_name() check NULL. It need not check for NULL 'again',
> > > > > > only once.
> > > > >
> > > > > Be so good to read the patch. I believe I've done so.
> > > > > However this prevents the detected enca message to appear for each
> > > > > language that is detected (some kind of debugging I guess).
> > > >
> > > > Sorry for didn't read it. it looks good. Only one language is
> > > > prefered, so the patch will change nothing about the message.
> > > >
> > > > >
> > > > > > Another place I think should fix is the end of the loop, should put a
> > > > > > break there.
> > > > >
> > > > > I'll let you do the commit for that. The break is missing in the old
> > > > > code and I am not sure if this is bug or it is intensional.
> > > >
> > > > Missing the break will affect nothing, add it just to make it clear
> > > > that only one language is matched. OK, I will make up the break one
> > > > day after your patch applied.
> > >
> > > First patch committed.
> > >
> > > Yes you are right about the break, it won't change anything as now we
> > > can check only for one language. I'll committing it together with my
> > > changes.
> >
> > Second patch committed, with the break change.
>
> I'd like put the break at just the end of loop, always break even got
> unknown charset since the loop only match one language.

I told you I'd like to have check for more than one language.

There is no need for break, the whole analyser code could be moved out
of the loop.  I prefer to keep it as it is, and use the current
indentation for the multiple languages check.