mate-desktop / mate-settings-daemon

MATE settings daemon
https://mate-desktop.org
GNU General Public License v2.0
43 stars 48 forks source link

daemon: Stop using deprecated GSettings API #235

Closed raveit65 closed 6 years ago

raveit65 commented 6 years ago

origin commit: https://gitlab.gnome.org/GNOME/gnome-settings-daemon/commit/36f32a3

Please test.

raveit65 commented 6 years ago

Why it should make more sense? Commit do exactly what glib2 deprecation warning do suggest.

mate-settings-manager.c: In Funktion »is_schema«:
mate-settings-manager.c:174:2: Warnung: »g_settings_list_schemas« ist veraltet: Use 'g_settings_schema_source_list_schemas' instead [-Wdeprecated-declarations]
  return is_item_in_schema (g_settings_list_schemas (), schema);
  ^~~~~~
In file included from /usr/include/glib-2.0/gio/gio.h:114,
                 from mate-settings-manager.c:34:
/usr/include/glib-2.0/gio/gsettings.h:74:25: Anmerkung: hier deklariert
 const gchar * const *   g_settings_list_schemas                         (void);
                         ^~~~~~~~~~~~~~~~~~~~~~~

So i think origin gnome commit use the right replacement for the deprecation..... ....... or not?

Honestly, i was very happy to find this commit which can be use as template, as we have this warning in other packages too.

vkareh commented 6 years ago

@raveit65 - you're right, I didn't think about this being a commit from gnome. I just saw this message in the deprecation notice:

If you used g_settings_list_schemas() to check for the presence of a particular schema, use g_settings_schema_source_lookup() instead of your whole loop.

https://developer.gnome.org/gio/stable/GSettings.html#g-settings-list-schemas

I'm fine either way, but whoever did this in gnome might have not known about the source_lookup function (or maybe it was added later?)

raveit65 commented 6 years ago

I'm fine either way

So, can you approve the PR? :)

muktupavels commented 6 years ago

No one needs to be expert, sometimes it is enough to read documentation!

[out][transfer full][array zero-terminated=1]

I guess that means you need to use g_strfreev...

raveit65 commented 6 years ago

What, there is leak? What should i do?

lukefromdc commented 6 years ago

I went to the docs for g_settings_schema_source_get_default and didn't get much guidance on this. Not sure how the others work but my guess would be the returned variables would be freed when the function calling them is done with them, unless they are pointers to something that remains in use and thus has other pointers to it.

muktupavels commented 6 years ago

https://developer.gnome.org/glib/stable/annotation-glossary.html

transfer full Free data after the code is done.

non_relocatable and relocatable are marked as transfer full so you should free them. Something like this:

in_schema = (is_item_in_schema (non_relocatable, schema) ||
             is_item_in_schema (relocatable, schema));

g_strfreev (non_relocatable);
g_strfreev (relocatable);

return in_schema;
lukefromdc commented 6 years ago

Just added the recommended fix for the memory leak, it worked as written

raveit65 commented 6 years ago

Thanks for fixing the leak and your reviews.