[MPlayer-dev-eng] [PATCH] libbs2b audio filter

[Andrew Savchenko]

Hi,

On Tuesday 24 March 2009 23:19, Reimar Döffinger wrote:
[...]
> I think I should for fairness clarify that I did not consider it
> _necessary_ for you to change it, I just wanted to clarify my
> opinion and the reasons for it.

I'm aware of this. But on second thought I agreed with your 
arguments.

> If there are no other comments/objections (in particular the
> build system part I did not really check) it can be applied IMO.
> Hm, I think there is some configure part missing?

I attached configure patch to one of previous e-mails. However, 
after recent configure changes by Diego patch should be updated 
due to renaming of some variables. New version is attached. I'll 
apply it after review together with two other patches: filter 
itself and documentation, posted earlier.

> > +    if (!(af->data = calloc(1,sizeof(af_data_t))))
> > +        return AF_ERROR;
> > +    if (!(af->setup = s = calloc(1,sizeof(af_bs2b_t)))) {
> > +        free(af->data);
> > +        return AF_ERROR;
> > +    }
> > +
> > +    // NULL means failed initialization
> > +    if (!(s->filter = bs2b_open())) {
> > +        free(af->data);
> > +        free(af->setup);
> > +        return AF_ERROR;
> > +    }
>
> It's fine by me (for now), but I'd like to point out that this
> approach is risky, if some new struct is added or things
> reordered or whatever you usually end up with a free missing.
> "goto error;" (while making sure that everything is properly
> (NULL-)initialized usually works well for this, as does in this
> case calling uninit().

But using goto operator in C code... whilst this is technically 
allowable I really dislike this kind of approach, it is ugly.

-- 
Best regards,
Andrew
-------------- next part --------------
A non-text attachment was scrubbed...
Name: libbs2b-configure-v3.patch
Type: text/x-diff
Size: 2766 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/mplayer-dev-eng/attachments/20090325/5a2b32ad/attachment.patch>
-------------- 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/mplayer-dev-eng/attachments/20090325/5a2b32ad/attachment.pgp>