Re: quartz: AMFilterData_ParseFilterData returns a pointer to a pointer to filter data

[Dmitry Timoshkov]

Aric Stewart <aric(a)codeweavers.com> wrote:
> -    REGFILTER2 *prf2;
> +    static REGFILTER2 *prf2;

What's the point of this change?

-- 
Dmitry.

quartz: AMFilterData_ParseFilterData returns a pointer to a pointer to filter data

[Chris Robinson]

On Friday, February 03, 2012 2:43:57 PM Dmitry Timoshkov wrote:
> Aric Stewart <aric(a)codeweavers.com> wrote:
> > -    REGFILTER2 *prf2;
> > +    static REGFILTER2 *prf2;
>
> What's the point of this change?

It returns a pointer to the pointer, so the variable needs to remain valid 
after the function exits. The code doesn't really feel safe, though. At the 
very least, I think the pointer should perhaps be made part of the 
implementation object, but even that feels kinda ugly. What happens if it's 
called multiple times?


REGFILTER **r1, **r2;

obj->ParseFilterData(data1, cb1, &r1);
obj->ParseFilterData(data2, cb2, &r2);

ok(r1 == r2, "Oops\n");

CoTaskMemFree(*r1);
/* *r2 is freed now, too */


Doesn't really seem right to me.

quartz: AMFilterData_ParseFilterData returns a pointer to a pointer to filter data

[Aric Stewart]

Yeah, I agree there is something odd happening with this function.

It clearly wants a pointer to a pointer,  and then graphedit and all the 
sample code I see online frees the inner pointer but never the outer 
pointer.

So I cannot alloc that outer pointer of we get a leak.  It does not help 
that the documentation for this method says "Normal applications will 
never call this function" And then proceeds to not describe it very well.

I felt like a static and an field in the object where roughly equivalent.

I can do some tests.
-aric

On 2/3/12 4:45 AM, Chris Robinson wrote:
> On Friday, February 03, 2012 2:43:57 PM Dmitry Timoshkov wrote:
> > Aric Stewart<aric(a)codeweavers.com>  wrote:
> > > -    REGFILTER2 *prf2;
> > > +    static REGFILTER2 *prf2;
> >
> > What's the point of this change?
>
> It returns a pointer to the pointer, so the variable needs to remain valid
> after the function exits. The code doesn't really feel safe, though. At the
> very least, I think the pointer should perhaps be made part of the
> implementation object, but even that feels kinda ugly. What happens if it's
> called multiple times?
>
> REGFILTER **r1, **r2;
>
> obj->ParseFilterData(data1, cb1,&r1);
> obj->ParseFilterData(data2, cb2,&r2);
>
> ok(r1 == r2, "Oops\n");
>
> CoTaskMemFree(*r1);
> /* *r2 is freed now, too */
>
> Doesn't really seem right to me.