Re: [AVIFIL32] question about ACMStream_fn[AddRef|Release] in acmstream.c

Paul Vriens  writes:
> Hi,
>
> during my code cleanup I came to acmstream.c and found the following:
>
> static ULONG WINAPI ACMStream_fnAddRef(IAVIStream *iface)
> {
>   IAVIStreamImpl *This = (IAVIStreamImpl *)iface;
>
> TRACE("(%p) -> %ld\n", iface, This->ref + 1);
>
> /* also add reference to the nested stream */
>   if (This->pStream != NULL)
>     IAVIStream_AddRef(This->pStream);
>
> return ++(This->ref);
> }
>
> the AddRef can be cleaned up.

I don't really know what you want to cleanup here, looks very clean to me.
> the Release however looks buggy:
>
> static ULONG WINAPI ACMStream_fnRelease(IAVIStream* iface)
> {
>   IAVIStreamImpl *This = (IAVIStreamImpl *)iface;
>
> TRACE("(%p) -> %ld\n", iface, This->ref - 1);
>
> if (This->ref == 0) {
>     /* destruct */
>
> This means I have to do a Release twice before it actually goes into the
> if statement, because the decrementing takes place at the end of the
> Release function.
>
> Can anybody confirm my finding.

You are right, the release method is buggy. the comparision must check
against one instead of zero or the decrement in the return statement
must be moved before this if-statement.

Michael Günnewig