[PATCH vkd3d v2 06/10] vkd3d-shader/hlsl: Properly free parse_variable_def memory in declare_vars().

Matteo Bruni matteo.mystral at gmail.com
Wed Jan 19 06:13:43 CST 2022


On Tue, Jan 11, 2022 at 7:44 PM Zebediah Figura (she/her)
<zfigura at codeweavers.com> wrote:
>
> On 1/11/22 12:23, Giovanni Mascellani wrote:
> > Hi,
> >
> > On 11/01/22 18:34, Zebediah Figura (she/her) wrote:
> >> I'm afraid I don't agree in this case. The principle is fine in
> >> general; the problem here is that struct parse_variable_def isn't
> >> semantically an object; it's a collection of information that's
> >> grouped into a struct only because yacc requires it. The fact that the
> >> code ends up using it in a relatively piecewise fashion is a good
> >> indication of this.
> >
> > It is of course a completely legitimate point of view, but I think it's
> > more prone to mistakes: for instance, the many missing calls to free()
> > that Francisco is fixing.
>
> I'm not convinced that treating it as an object would really help here.
> Leaks are just hard.
>
> > Also, if we insist in keeping this point of view, then we should remove
> > free_parse_variable_def(), because that's precisely one of the gadgets
> > that indicates that the structure is meant to be an encapsulated object.
>
> I don't see anything wrong with having helpers like that.

I tend to lean towards Zeb's view on both points.



More information about the wine-devel mailing list