Re: [Vala] Pango.Attribute questions

[Abderrahim Kitouni <a kitouni gmail com>]

Hi,

2009/11/6 pHilipp Zabel <philipp zabel gmail com>:
> Hi,
>
> I'm a bit confused about how Pango.Attribute is supposed to work,
> probable because I don't understand ownership issues of [Compact]
> classes properly.
>
> I want to set some Pango attributes on a Gtk.CellRendererText, so I tried this:
>
> 1:      Pango.AttrList attr_list = new Pango.AttrList ();
> 2:      Pango.Attribute attr_color = Pango.attr_foreground_new (color.red,
> color.green, color.blue);
> 3:      Pango.Attribute attr_scale = Pango.attr_scale_new (Pango.Scale.SMALL);
> 4:      attr_list.insert (attr_color);
> 5:      attr_list.insert (attr_scale);
> 6:      renderer.attributes = attr_list;
>
> Vala complains with an "error: duplicating Attribute instance, use
> unowned variable or explicitly invoke copy method" in lines 2, 3, 4
> and 5.

This sounds like a bug in the vapi, there is no reason you should be
duplicating instances here : if I understand correctly
attr_forground_new and attr_scale_new are actually constructors of
Pango.Attribute, so you should own the returned reference (and hence
not need to copy it or use an unowned variable).
I'm not sure about insert but unless it takes ownership of the passed
attribute (i.e. you don't need to free it after you pass it), you
don't need to copy it. If it does take ownership, another trick (not
mentioned in the error message because it doesn't work in all
situations is to use an '(owned)' cast, i.e. attr_list.insert ((owned)
attr_color); this way, attr_list is now responsible for freeing
attr_color, and your attr_color is set to null).


> This compiles to the following C code:
>
> 1:      attr_list = pango_attr_list_new ();
> 2:      attr_color = pango_attr_foreground_new (color.red, color.green, color.blue);
> 3:      attr_scale = pango_attr_scale_new (PANGO_SCALE_SMALL);
> 4:      pango_attr_list_insert (attr_list, pango_attribute_copy (attr_color));
> 5:      pango_attr_list_insert (attr_list, pango_attribute_copy (attr_scale));
> 6:      g_object_set (renderer, "attributes", attr_list, NULL);
>
> Am I not leaking memory for attr_color and attr_scale due to the copy
> and the fact that pango_attribute_destroy is never called on either of
> them after the copies are made?
> I'd expect the correct C code to look something like this:
>
> 1:      attr_list = pango_attr_list_new ();
> 2:      attr_color = pango_attr_foreground_new (color.red, color.green, color.blue);
> 3:      attr_scale = pango_attr_scale_new (PANGO_SCALE_SMALL);
> 4:      pango_attr_list_insert (attr_list, attr_color);
> 5:      pango_attr_list_insert (attr_list, attr_scale);
> 6:      g_object_set (renderer, "attributes", attr_list, NULL);

so the problem is that attr_forground_new and attr_scale_new should
not be returning unowned references (I think they should be
constructors).
(and you should use either owned cast or copy depending on whether you
wan to use it later or not)

HTH,
Abderrahim