Eli Zaretskii <eliz@gnu.org> writes:

>> The main memory leak, though, was that the MonitorList array wasn't
>> being freed.
>
> Does your patch change that?  If not, why not?

It did, yes. That was the xfree (monitors) call.

>> I considered using the free_monitors procedure like the non-GTK
>> versions do, but I saw no reason to call g_strdup for each name and
>> free each name almost right after.
>
> I don't see how the short lifetime of the array changes anything
> here.  As long as we aren't sure the pointer returned by
> gdk_monitor_get_model is a copy that cannot be changed by another
> thread, we should ourselves make a copy.  Otherwise, who can ensure us
> that some other GTK thread doesn't call this same function during the
> short life time of the array?

The documentation does state that the name property of the monitor is
read-only.

>> Since make_monitor_attribute_list uses make_string on each name, I don't
>> see any issues that this removal would cause.
>
> The issue I see is that between the time we call gdk_monitor_get_model
> and the time we use the string something could change the string to
> which the pointer points.  If you can spot something in the GDK docs
> that guarantees this couldn't happen, please point me to that piece of
> docs.

Well, I suppose that the monitor could be unplugged in-between, which
presumably would mean that the monitor object is freed. Would that space
be reused in-between, though?

I suppose it doesn't hurt to play it safe, so I updated the patch to use
free_monitors instead: