After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 566706 - cleanup GIO overrides
cleanup GIO overrides
Status: RESOLVED FIXED
Product: pygobject
Classification: Bindings
Component: gio
Git master
Other All
: Normal normal
: ---
Assigned To: Paul Pogonyshev
Python bindings maintainers
Depends on:
Blocks:
 
 
Reported: 2009-01-05 23:47 UTC by Paul Pogonyshev
Modified: 2009-01-14 18:40 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
patch to clean up GIO overrides (43.03 KB, patch)
2009-01-06 00:02 UTC, Paul Pogonyshev
committed Details | Review

Description Paul Pogonyshev 2009-01-05 23:47:29 UTC
Attached huge patch cleans up GIO override functions.  Patch with description follows.
Comment 1 Paul Pogonyshev 2009-01-06 00:02:30 UTC
Created attachment 125810 [details] [review]
patch to clean up GIO overrides

This patch aims at reducing code repetition in GIO overriding functions.  I don't need to list arguments against code repetition, they are standard.  And in our case code repetition _did_ cause many bugs (though all minor).  Additionally, this patch makes PyGIONotify structure more like a class with methods.  Instead of "randomly" referencing its fields, we now call methods.  Fields are of course visible (this is C), but only reading is done directly now.  Finally, I added a few safeguards for programmers, e.g. if you make an error while programming an override, there is a chance pygio_notify_*() functions will notice it and give you a helpful message.

Note that only real argument against such changes --- performance --- doesn't apply, because performance reduction is absolutely negligible given input/output GIO functions.

Bugs I noticed and fixed:

* Almost all functions leaked memory and Python references if an error in argument validation happened.

* Many functions did not check if 'callback' was callable while others did.

List of notable changes:

* New functions pygio_notify_new(), pygio_notify_using_optional_callback(), pygio_notify_callback_is_valid_full(), pygio_notify_callback_is_valid(), pygio_notify_reference_callback().

* Rename pygio_free_notify() to pygio_notify_free().

* Use the new functions everywhere.

* Instead of constantly repeating (and buggy!) error cleanup code, use 'goto error' and write cleanup code only once per function.

* Tons of minor bug-fixes (see above).

* Fix the recently added test for gio.File.copy_async().

All relevant unit tests pass.
Comment 2 Paul Pogonyshev 2009-01-14 18:40:18 UTC
I'll just go ahead and commit this.

Sending        ChangeLog
Sending        gio/gfile.override
Sending        gio/gfileenumerator.override
Sending        gio/gicon.override
Sending        gio/ginputstream.override
Sending        gio/gio.override
Sending        gio/goutputstream.override
Sending        gio/gvolume.override
Sending        tests/test_gio.py
Transmitting file data .........
Committed revision 996.


2009-01-14  Paul Pogonyshev  <pogonyshev@gmx.net>

	Bug 566706 – cleanup GIO overrides

	* gio/gio.override (pygio_notify_new)
	(pygio_notify_using_optional_callback)
	(pygio_notify_callback_is_valid_full)
	(pygio_notify_callback_is_valid)
	(pygio_notify_reference_callback): New functions.
	(pygio_notify_free): Rename from pygio_free_notify() and extend.
	(async_result_callback_marshal): Warn if new `referenced' field is
	not set (programming error).
	(_wrap_g_drive_eject, _wrap_g_drive_poll_for_media)
	(_wrap_g_mount_unmount, _wrap_g_mount_eject)
	(_wrap_g_mount_remount): Lots of cleanup: use new functions
	instead of repeating code, unify and fix error handling.

	* gio/gfile.override (_wrap_g_file_read_async)
	(_wrap_g_file_load_contents_async)
	(_wrap_g_file_enumerate_children_async)
	(_wrap_g_file_mount_mountable, _wrap_g_file_unmount_mountable)
	(_wrap_g_file_mount_enclosing_volume, _wrap_g_file_copy)
	(_wrap_g_file_copy_async, _wrap_g_file_move)
	(_wrap_g_file_append_to_async, _wrap_g_file_create_async)
	(_wrap_g_file_replace_async, _wrap_g_file_query_info_async)
	(_wrap_g_file_replace_contents_async): Similar cleanup.

	* gio/gfileenumerator.override
	(_wrap_g_file_enumerator_next_files_async): Similar cleanup.

	* gio/gicon.override (_wrap_g_loadable_icon_load_async): Similar
	cleanup.

	* gio/ginputstream.override (_wrap_g_input_stream_close_async):
	Similar cleanup.

	* gio/goutputstream.override (_wrap_g_output_stream_write_async)
	(_wrap_g_output_stream_close_async): Similar cleanup.

	* gio/gvolume.override (_wrap_g_volume_mount)
	(_wrap_g_volume_eject): Similar cleanup.

	* tests/test_gio.py (TestFile.test_copy_async): Fix the test.