GNOME Bugzilla – Bug 666115
various tests leak memory, obscuring real leaks in the library
Last modified: 2018-05-24 13:37:07 UTC
If Bug #666114 is fixed, real leaks like those in Bug #666113 are made harder to see among harmless leaks and other misbehaviour from the tests themselves. Patches to follow.
Created attachment 203378 [details] [review] tls-interaction test: use a weak pointer instead of a deliberate use-after-free --- The intention seems to be to check that the object is actually finalized; weak pointers seem a better way to do that.
Created attachment 203379 [details] [review] testglib: test_file_functions: don't close fd if it's -1 --- Causes harmless noise from valgrind.
Created attachment 203380 [details] [review] various GLib tests: plug memory leaks --- These are the relatively easy/obvious ones; if any of them are controversial, I'll split them into a separate commit.
Created attachment 203381 [details] [review] GOptionContext test: free all arguments, not just the remaining ones On success, g_option_context_parse alters argv by removing options that it understood, so g_strfreev is insufficient. Instead, take a shallow copy and free all of the arguments in that, then free the array argv but not its contents. Also, improve the checks in error cases, by checking that argv has not been altered in this way.
Created attachment 203382 [details] [review] hash test: avoid leaking various keys and values
Created attachment 203383 [details] [review] Plug some leaks in the GIO tests
Review of attachment 203380 [details] [review]: Wow, quite a lot of work here, thanks. I'll just assume that these are all fine...
Review of attachment 203379 [details] [review]: Sure
Review of attachment 203381 [details] [review]: hmm, ok
Review of attachment 203382 [details] [review]: quite a bit more complicated. but ok, I guess, in the name of leakfreeness
Review of attachment 203383 [details] [review]: ok
(In reply to comment #1) > Created an attachment (id=203378) > tls-interaction test: use a weak pointer instead of a deliberate > use-after-free Thanks for reviewing, I'll get those committed. Any thoughts on this one?
(In reply to comment #12) > Thanks for reviewing, I'll get those committed. All patches except Attachment #203378 [details] pushed to master.
Comment on attachment 203378 [details] [review] tls-interaction test: use a weak pointer instead of a deliberate use-after-free yeah, I fixed a bunch of these the same way in glib-networking too. (Though I was thinking maybe it would be nice to have a g_object_assert_last_unref() macro maybe...)
Created attachment 203492 [details] [review] Add a g_object_assert_last_unref macro --- (In reply to comment #14) > (From update of attachment 203378 [details] [review]) > yeah, I fixed a bunch of these the same way in glib-networking too. Thanks, pushed as a1bd6e0717. > (Though I > was thinking maybe it would be nice to have a g_object_assert_last_unref() > macro maybe...) Here you go!
Comment on attachment 203492 [details] [review] Add a g_object_assert_last_unref macro just noticed this lying around... looks good to me
Review of attachment 203492 [details] [review]: ::: gobject/gobject.h @@ +587,3 @@ + * this macro just calls g_object_unref() without any further checks. + * + * This macro should only be used in regression tests. Since: 2.34 @@ +597,3 @@ + gpointer _weak_pointer = (object); \ + \ + g_assert (G_IS_OBJECT (_weak_pointer)); \ Do we need this assertion? g_object_add_weak_pointer() repeats it.
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/glib/issues/488.