GNOME Bugzilla – Bug 756766
Leak fixes
Last modified: 2017-09-15 09:52:53 UTC
Here is a patch series fixing various leaks I found running valgrind on the test suite.
Created attachment 313575 [details] [review] build-sys: Update valgrind headers libsecret ships headers from valgrind, but they seem to come from an old version. Update to the headers from valgrind-3.11.0-1.fc23.x86_64 in order to get VG_DO_ADDED_LEAK_CHECK definition.
Created attachment 313576 [details] [review] secret-item: Fix 'convenince' typo
Created attachment 313577 [details] [review] Fix 'attributes' typo in API doc
Created attachment 313578 [details] [review] test-item: Fix GError leak This fixes: ==16901== 94 (16 direct, 78 indirect) bytes in 1 blocks are definitely lost in loss record 901 of 1,154 ==16901== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16901== by 0x36E564F679: g_malloc (gmem.c:97) ==16901== by 0x36E5666CD2: g_slice_alloc (gslice.c:1007) ==16901== by 0x36E563522B: g_error_new_valist (gerror.c:386) ==16901== by 0x36E563560A: g_set_error (gerror.c:560) ==16901== by 0x4C2CFB2: secret_item_initable_init (secret-item.c:480) ==16901== by 0x36E6E5D83E: g_initable_new_valist (ginitable.c:228) ==16901== by 0x36E6E5D8F5: g_initable_new (ginitable.c:146) ==16901== by 0x4C3E711: secret_item_new_for_dbus_path_sync (secret-paths.c:286) ==16901== by 0x402506: test_new_sync_noexist (test-item.c:121) ==16901== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16901== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16901== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16901== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16901== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16901== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16901== by 0x406AAE: main (test-item.c:880)
Created attachment 313579 [details] [review] test-item: Fix GHashTable leak This fixes: ==16901== 248 (88 direct, 160 indirect) bytes in 1 blocks are definitely lost in loss record 1,108 of 1,154 ==16901== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16901== by 0x36E564F679: g_malloc (gmem.c:97) ==16901== by 0x36E5666CD2: g_slice_alloc (gslice.c:1007) ==16901== by 0x36E563860D: g_hash_table_new_full (ghash.c:711) ==16901== by 0x4047B2: test_set_attributes_async (test-item.c:493) ==16901== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16901== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16901== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16901== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16901== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16901== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16901== by 0x406AAE: main (test-item.c:880)
Created attachment 313580 [details] [review] test-item: Fix string leaks This fixes: ==16901== 7 bytes in 1 blocks are definitely lost in loss record 9 of 1,154 ==16901== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16901== by 0x36E564F679: g_malloc (gmem.c:97) ==16901== by 0x36E56688AE: g_strdup (gstrfuncs.c:356) ==16901== by 0x4C3098F: secret_item_get_label (secret-item.c:1922) ==16901== by 0x402A65: test_create_sync (test-item.c:203) ==16901== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16901== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16901== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16901== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16901== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16901== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16901== by 0x406AAE: main (test-item.c:880) ==16901==- ==16901== 7 bytes in 1 blocks are definitely lost in loss record 10 of 1,154 ==16901== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16901== by 0x36E564F679: g_malloc (gmem.c:97) ==16901== by 0x36E56688AE: g_strdup (gstrfuncs.c:356) ==16901== by 0x4C3098F: secret_item_get_label (secret-item.c:1922) ==16901== by 0x402DB6: test_create_async (test-item.c:249) ==16901== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16901== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16901== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16901== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16901== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16901== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16901== by 0x406AAE: main (test-item.c:880) ==16901==-
Created attachment 313581 [details] [review] item: Free SecreItem::value in finalize() This fixes: ==20768== 67 (40 direct, 27 indirect) bytes in 1 blocks are definitely lost in loss record 1,133 of 1,588 ==20768== at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==20768== by 0x6002FCC: g_malloc (gmem.c:94) ==20768== by 0x601B523: g_slice_alloc (gslice.c:1007) ==20768== by 0x601B563: g_slice_alloc0 (gslice.c:1032) ==20768== by 0x4E5FB64: secret_value_new_full (secret-value.c:140) ==20768== by 0x4E67B63: service_decode_aes_secret (secret-session.c:457) ==20768== by 0x4E67D6B: _secret_session_decode_secret (secret-session.c:513) ==20768== by 0x4E50A61: on_item_load_secret (secret-item.c:1157) ==20768== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==20768== by 0x5A33575: g_task_return (gtask.c:1162) ==20768== by 0x5A33E76: g_task_return_pointer (gtask.c:1537) ==20768== by 0x5AAC7E6: reply_cb (gdbusproxy.c:2579) ==20768== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==20768== by 0x5A33575: g_task_return (gtask.c:1162) ==20768== by 0x5A33E76: g_task_return_pointer (gtask.c:1537) ==20768== by 0x5A9BB7D: g_dbus_connection_call_done (gdbusconnection.c:5704) ==20768== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==20768== by 0x5A334B6: complete_in_idle_cb (gtask.c:1118) ==20768== by 0x5FFD3D0: g_idle_dispatch (gmain.c:5441) ==20768== by 0x5FFAA18: g_main_dispatch (gmain.c:3154) ==20768== by 0x5FFB85C: g_main_context_dispatch (gmain.c:3769) ==20768== by 0x5FFBA40: g_main_context_iterate (gmain.c:3840) ==20768== by 0x5FFBE66: g_main_loop_run (gmain.c:4034) ==20768== by 0x4E510DA: secret_item_load_secret_sync (secret-item.c:1311) ==20768== by 0x405238: test_load_secret_sync (test-item.c:592) ==20768== by 0x60258FA: test_case_run (gtestutils.c:2158) ==20768== by 0x6025CBB: g_test_run_suite_internal (gtestutils.c:2241) ==20768== by 0x6025D64: g_test_run_suite_internal (gtestutils.c:2253) ==20768== by 0x6025F7B: g_test_run_suite (gtestutils.c:2328) ==20768== by 0x6024C1C: g_test_run (gtestutils.c:1596) ==20768== by 0x4E7CA25: egg_tests_run_with_loop (egg-testing.c:167) ==20768== by 0x406B52: main (test-item.c:887)
Created attachment 313582 [details] [review] item: Fix local variable leak in on_create_path() This fixes: ==22678== 48 bytes in 1 blocks are definitely lost in loss record 1,045 of 1,560 ==22678== at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==22678== by 0x6002FCC: g_malloc (gmem.c:94) ==22678== by 0x60032AE: g_malloc_n (gmem.c:330) ==22678== by 0x601DBF0: g_strdup (gstrfuncs.c:363) ==22678== by 0x4E6508B: on_create_item_called (secret-paths.c:1991) ==22678== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==22678== by 0x5A33575: g_task_return (gtask.c:1162) ==22678== by 0x5A33E76: g_task_return_pointer (gtask.c:1537) ==22678== by 0x5A9BB7D: g_dbus_connection_call_done (gdbusconnection.c:5704) ==22678== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==22678== by 0x5A334B6: complete_in_idle_cb (gtask.c:1118) ==22678== by 0x5FFD3D0: g_idle_dispatch (gmain.c:5441) ==22678== by 0x5FFAA18: g_main_dispatch (gmain.c:3154) ==22678== by 0x5FFB85C: g_main_context_dispatch (gmain.c:3769) ==22678== by 0x5FFBA40: g_main_context_iterate (gmain.c:3840) ==22678== by 0x5FFBE66: g_main_loop_run (gmain.c:4034) ==22678== by 0x4E7C9E4: loop_wait_until (egg-testing.c:151) ==22678== by 0x4E7C85C: egg_test_wait_until (egg-testing.c:105) ==22678== by 0x402D22: test_create_async (test-item.c:245) ==22678== by 0x60258FA: test_case_run (gtestutils.c:2158) ==22678== by 0x6025CBB: g_test_run_suite_internal (gtestutils.c:2241) ==22678== by 0x6025D64: g_test_run_suite_internal (gtestutils.c:2253) ==22678== by 0x6025F7B: g_test_run_suite (gtestutils.c:2328) ==22678== by 0x6024C1C: g_test_run (gtestutils.c:1596) ==22678== by 0x4E7CA3E: egg_tests_run_with_loop (egg-testing.c:167) ==22678== by 0x406B52: main (test-item.c:887) =
Created attachment 313583 [details] [review] Fix secret_schema_unref detection of 0 refcount g_atomic_int_add (&schema->refs, -1); will return the value of 'refs' _before_ adding -1 to it, so checking this value for 0 to see if the refcount dropped to 0 after adding -1 is not going to work and will cause a leak. Using g_atomic_int_dec_and_test() fixes this problem as this will return TRUE when the value drops to 0 after being decremented.
Created attachment 313584 [details] [review] test-prompt: Fix GError leak
Created attachment 313585 [details] [review] prompt: Don't leak 'owner_name' in secret_prompt_perform() This fixes: ==16832== 7 bytes in 1 blocks are definitely lost in loss record 10 of 1,184 ==16832== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16832== by 0x36E564F679: g_malloc (gmem.c:97) ==16832== by 0x36E56688AE: g_strdup (gstrfuncs.c:356) ==16832== by 0x36E6EDEB98: g_dbus_proxy_get_name_owner (gdbusproxy.c:2372) ==16832== by 0x4C38B64: secret_prompt_perform (secret-prompt.c:468) ==16832== by 0x4C39F4C: secret_service_real_prompt_async (secret-service.c:339) ==16832== by 0x4C3D126: secret_service_prompt (secret-service.c:1711) ==16832== by 0x4C42182: on_create_collection_called (secret-paths.c:1706) ==16832== by 0x36E6E74BA6: g_simple_async_result_complete (gsimpleasyncresult.c:763) ==16832== by 0x36E6ED1921: g_dbus_connection_call_done (gdbusconnection.c:5508) ==16832== by 0x36E6E74BA6: g_simple_async_result_complete (gsimpleasyncresult.c:763) ==16832== by 0x36E6E74C08: complete_in_idle_cb (gsimpleasyncresult.c:775) ==16832== by 0x36E5649A89: g_main_dispatch (gmain.c:3122) ==16832== by 0x36E5649A89: g_main_context_dispatch (gmain.c:3737) ==16832== by 0x36E5649E1F: g_main_context_iterate.isra.29 (gmain.c:3808) ==16832== by 0x36E564A141: g_main_loop_run (gmain.c:4002) ==16832== by 0x4C427F1: secret_service_create_collection_dbus_path_sync (secret-paths.c:1908) ==16832== by 0x4C299F9: secret_collection_create_sync (secret-collection.c:1211) ==16832== by 0x403084: test_create_sync (test-collection.c:299) ==16832== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16832== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16832== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16832== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16832== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16832== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16832== by 0x4068B0: main (test-collection.c:1028)
Created attachment 313586 [details] [review] test-collection: Fix GError leaks This fixes (among others): ==16832== 95 (16 direct, 79 indirect) bytes in 1 blocks are definitely lost in loss record 927 of 1,184 ==16832== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16832== by 0x36E564F679: g_malloc (gmem.c:97) ==16832== by 0x36E5666CD2: g_slice_alloc (gslice.c:1007) ==16832== by 0x36E563522B: g_error_new_valist (gerror.c:386) ==16832== by 0x36E563560A: g_set_error (gerror.c:560) ==16832== by 0x4C27FB4: secret_collection_initable_init (secret-collection.c:543) ==16832== by 0x36E6E5D83E: g_initable_new_valist (ginitable.c:228) ==16832== by 0x36E6E5D8F5: g_initable_new (ginitable.c:146) ==16832== by 0x4C3E1E3: secret_collection_new_for_dbus_path_sync (secret-paths.c:158) ==16832== by 0x404733: test_delete_sync (test-collection.c:623) ==16832== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16832== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16832== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16832== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16832== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16832== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16832== by 0x4068B0: main (test-collection.c:1028)
Created attachment 313587 [details] [review] test-collection: Fix string leaks This fixes: ==16832== 6 bytes in 1 blocks are definitely lost in loss record 8 of 1,184 ==16832== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16832== by 0x36E564F679: g_malloc (gmem.c:97) ==16832== by 0x36E56688AE: g_strdup (gstrfuncs.c:356) ==16832== by 0x4C2B242: secret_collection_get_label (secret-collection.c:1835) ==16832== by 0x40313D: test_create_sync (test-collection.c:305) ==16832== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16832== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16832== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16832== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16832== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16832== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16832== by 0x4068B0: main (test-collection.c:1028) ==16832==- ==16832== 6 bytes in 1 blocks are definitely lost in loss record 9 of 1,184 ==16832== at 0x4A06C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==16832== by 0x36E564F679: g_malloc (gmem.c:97) ==16832== by 0x36E56688AE: g_strdup (gstrfuncs.c:356) ==16832== by 0x4C2B242: secret_collection_get_label (secret-collection.c:1835) ==16832== by 0x40337C: test_create_async (test-collection.c:333) ==16832== by 0x36E566FB92: test_case_run (gtestutils.c:2124) ==16832== by 0x36E566FB92: g_test_run_suite_internal (gtestutils.c:2185) ==16832== by 0x36E566FD5A: g_test_run_suite_internal (gtestutils.c:2196) ==16832== by 0x36E56700DA: g_test_run_suite (gtestutils.c:2249) ==16832== by 0x36E5670110: g_test_run (gtestutils.c:1553) ==16832== by 0x4C5A0EA: egg_tests_run_with_loop (egg-testing.c:167) ==16832== by 0x4068B0: main (test-collection.c:1028)
Created attachment 313588 [details] [review] collection: Fix on_create_path() leak This fixes: ==2111== 45 bytes in 1 blocks are definitely lost in loss record 1,180 of 1,795 ==2111== at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==2111== by 0x6002FCC: g_malloc (gmem.c:94) ==2111== by 0x60032AE: g_malloc_n (gmem.c:330) ==2111== by 0x601DBF0: g_strdup (gstrfuncs.c:363) ==2111== by 0x603C073: g_variant_dup_string (gvariant.c:1503) ==2111== by 0x4E644D5: on_create_collection_prompt (secret-paths.c:1682) ==2111== by 0x5A1CB66: g_simple_async_result_complete (gsimpleasyncresult.c:801) ==2111== by 0x4E5C12F: on_real_prompt_completed (secret-service.c:322) ==2111== by 0x5A1CB66: g_simple_async_result_complete (gsimpleasyncresult.c:801) ==2111== by 0x4E5A6A1: perform_prompt_complete (secret-prompt.c:288) ==2111== by 0x4E5A7B5: on_prompt_completed (secret-prompt.c:314) ==2111== by 0x5A97B7B: emit_signal_instance_in_idle_cb (gdbusconnection.c:3701) ==2111== by 0x5FFD3D0: g_idle_dispatch (gmain.c:5441) ==2111== by 0x5FFAA18: g_main_dispatch (gmain.c:3154) ==2111== by 0x5FFB85C: g_main_context_dispatch (gmain.c:3769) ==2111== by 0x5FFBA40: g_main_context_iterate (gmain.c:3840) ==2111== by 0x5FFBE66: g_main_loop_run (gmain.c:4034) ==2111== by 0x4E7C9E1: loop_wait_until (egg-testing.c:151) ==2111== by 0x4E7C859: egg_test_wait_until (egg-testing.c:105) ==2111== by 0x4032ED: test_create_async (test-collection.c:328) ==2111== by 0x60258FA: test_case_run (gtestutils.c:2158) ==2111== by 0x6025CBB: g_test_run_suite_internal (gtestutils.c:2241) ==2111== by 0x6025D64: g_test_run_suite_internal (gtestutils.c:2253) ==2111== by 0x6025F7B: g_test_run_suite (gtestutils.c:2328) ==2111== by 0x6024C1C: g_test_run (gtestutils.c:1596) ==2111== by 0x4E7CA3B: egg_tests_run_with_loop (egg-testing.c:167) ==2111== by 0x406948: main (test-collection.c:1033)
Created attachment 313589 [details] [review] Don't leak ItemClosure::collection_path This fixes: ==17724== 48 bytes in 1 blocks are definitely lost in loss record 1,135 of 1,698 ==17724== at 0x4C28C50: malloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==17724== by 0x6002FCC: g_malloc (gmem.c:94) ==17724== by 0x60032AE: g_malloc_n (gmem.c:330) ==17724== by 0x601DBF0: g_strdup (gstrfuncs.c:363) ==17724== by 0x4E650B5: on_create_item_called (secret-paths.c:1990) ==17724== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==17724== by 0x5A33575: g_task_return (gtask.c:1162) ==17724== by 0x5A33E76: g_task_return_pointer (gtask.c:1537) ==17724== by 0x5A9BB7D: g_dbus_connection_call_done (gdbusconnection.c:5704) ==17724== by 0x5A3346D: g_task_return_now (gtask.c:1104) ==17724== by 0x5A334B6: complete_in_idle_cb (gtask.c:1118) ==17724== by 0x5FFD3D0: g_idle_dispatch (gmain.c:5441) ==17724== by 0x5FFAA18: g_main_dispatch (gmain.c:3154) ==17724== by 0x5FFB85C: g_main_context_dispatch (gmain.c:3769) ==17724== by 0x5FFBA40: g_main_context_iterate (gmain.c:3840) ==17724== by 0x5FFBE66: g_main_loop_run (gmain.c:4034) ==17724== by 0x4E55FF7: secret_service_store_sync (secret-methods.c:1281) ==17724== by 0x404AC8: test_store_sync (test-methods.c:736) ==17724== by 0x60258FA: test_case_run (gtestutils.c:2158) ==17724== by 0x6025CBB: g_test_run_suite_internal (gtestutils.c:2241) ==17724== by 0x6025D64: g_test_run_suite_internal (gtestutils.c:2253) ==17724== by 0x6025F7B: g_test_run_suite (gtestutils.c:2328) ==17724== by 0x6024C1C: g_test_run (gtestutils.c:1596) ==17724== by 0x4E7CA68: egg_tests_run_with_loop (egg-testing.c:167) ==17724== by 0x406190: main (test-methods.c:998)
Created attachment 313590 [details] [review] Free CollectionClosure::collection_path when it's freed Similarly to what is done ItemClosure::collection_path, make sure that CollectionClosure::collection_path is freed in collection_closure_free().
Created attachment 313591 [details] [review] secret-sync: Fix SecretSync leak The memory allocated for the SecretSync instance itself was not freed. This fixes: ==7106== 24 bytes in 1 blocks are definitely lost in loss record 855 of 1,629 ==7106== at 0x4C2A9C7: calloc (in /usr/lib64/valgrind/vgpreload_memcheck-amd64-linux.so) ==7106== by 0x600303F: g_malloc0 (gmem.c:124) ==7106== by 0x6003322: g_malloc0_n (gmem.c:354) ==7106== by 0x4E6979D: _secret_sync_new (secret-util.c:441) ==7106== by 0x4E65D11: secret_service_read_alias_dbus_path_sync (secret-paths.c:2334) ==7106== by 0x4E4E09E: secret_collection_for_alias_sync (secret-collection.c:2221) ==7106== by 0x40270E: test_for_alias_sync (test-collection.c:187) ==7106== by 0x60258FA: test_case_run (gtestutils.c:2158) ==7106== by 0x6025CBB: g_test_run_suite_internal (gtestutils.c:2241) ==7106== by 0x6025D64: g_test_run_suite_internal (gtestutils.c:2253) ==7106== by 0x6025F7B: g_test_run_suite (gtestutils.c:2328) ==7106== by 0x6024C1C: g_test_run (gtestutils.c:1596) ==7106== by 0x4E7CA3B: egg_tests_run_with_loop (egg-testing.c:167) ==7106== by 0x406948: main (test-collection.c:1033)
Review of attachment 313575 [details] [review]: This results in the following warnings: In file included from ../egg/egg-secure-memory.c:45:0: ../egg/egg-secure-memory.c: In function ‘memcpy_with_vbits’: ../build/valgrind/memcheck.h:283:5: warning: value computed is not used [-Wunused-value] (unsigned)VALGRIND_DO_CLIENT_REQUEST_EXPR(0, \ ^ ../egg/egg-secure-memory.c:658:3: note: in expansion of macro ‘VALGRIND_SET_VBITS’ VALGRIND_SET_VBITS (dest, vbits, length); ^ ../build/valgrind/memcheck.h:283:5: warning: value computed is not used [-Wunused-value] (unsigned)VALGRIND_DO_CLIENT_REQUEST_EXPR(0, \ ^ ../egg/egg-secure-memory.c:659:3: note: in expansion of macro ‘VALGRIND_SET_VBITS’ VALGRIND_SET_VBITS (src, vbits, length); ^ Once those are fixed, LGTM.
Review of attachment 313576 [details] [review]: Looks good to me.
Review of attachment 313577 [details] [review]: Looks good to me.
Review of attachment 313578 [details] [review]: Looks good to me.
Review of attachment 313579 [details] [review]: Looks good to me.
Review of attachment 313580 [details] [review]: Looks good to me.
Review of attachment 313581 [details] [review]: Looks good to me.
Review of attachment 313582 [details] [review]: Looks good to me.
Review of attachment 313583 [details] [review]: This patch removes a programmer error diagnostic. We should produce a precondition style g_warning() in the case where the caller has passed in a statically allocated or invalid SecretSchema structure.
Review of attachment 313584 [details] [review]: Looks good to me.
Review of attachment 313585 [details] [review]: Looks good to me.
Review of attachment 313586 [details] [review]: Looks good to me, but could you add a space after the function call before the parentheses.
Review of attachment 313587 [details] [review]: Looks good to me.
Review of attachment 313588 [details] [review]: Looks good to me.
Review of attachment 313589 [details] [review]: Looks good to me.
Review of attachment 313590 [details] [review]: Looks good to me.
Review of attachment 313591 [details] [review]: Looks good to me.
Thanks for the work on this. Wonderful :)
Created attachment 313651 [details] [review] build-sys: Update valgrind headers libsecret ships headers from valgrind, but they seem to come from an old version. Update to the headers from valgrind-3.11.0-1.fc23.x86_64 in order to get VG_DO_ADDED_LEAK_CHECK definition.
Created attachment 313652 [details] [review] Fix secret_schema_unref detection of 0 refcount g_atomic_int_add (&schema->refs, -1); will return the value of 'refs' _before_ adding -1 to it, so checking this value for 0 to see if the refcount dropped to 0 after adding -1 is not going to work and will cause a leak. Using g_atomic_int_dec_and_test() fixes this problem as this will return TRUE when the value drops to 0 after being decremented.
Review of attachment 313651 [details] [review]: Thanks. Looks good to me.
Review of attachment 313652 [details] [review]: Looks good. Thanks.
*** Bug 736738 has been marked as a duplicate of this bug. ***