On Tue, Jan 11, 2022 at 7:44 PM Zebediah Figura (she/her) zfigura@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.