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

Reimar Döffinger Reimar.Doeffinger at gmx.de
Tue Mar 24 21:19:19 CET 2009


On Mon, Mar 23, 2009 at 11:19:23PM +0300, Andrew Savchenko wrote:
> > I complained 
> > about the checks like you did them because while they are little
> > code, the gains are even less IMHO, and both no code or a little
> > bit more code to do it right seem like much more sensible in the
> > effort vs. gain comparison.
> 
> Then I choose little more code ;-)

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.
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?

> +    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().



More information about the MPlayer-dev-eng mailing list