[Geany-devel] Changed file saving implementation for systems with GIO

[Colomban Wendling]

Le 04/11/2010 17:26, Nick Treleaven a écrit :
> On Wed, 03 Nov 2010 00:26:00 +0100
> Colomban Wendling <lists.ban at herbesfolles.org> wrote:
> 
>> Le 02/11/2010 19:11, Nick Treleaven a écrit :
>>> Hi list,
>>> To workaround bug(s) in GVFS, I've now changed the file saving
>>> implementation for systems with GIO in SVN trunk. See also:
>>>
>>> http://lists.uvena.de/geany-devel/2010-November/003412.html
>>>
>>> Please help test this. File permissions should be preserved after
>>> saving, and disk space exhaustion should now be handled without losing
>>> the previously saved file.
>> Interesting change, let's see what happens now -- this said I don't use
>> remote files often so I will probably not notice any change...
> 
> Actually I should have been clearer - this change affects local files
> too. I'll list the fixes now:
I should have been clearer too -- what I meant is that since I use local
files, I don't face issues that GVFS (-fuse?) may have, so as far as the
file saving still works, I woudln't notice :)

>> Just to mention (I just saw that the question raised in the original
>> thread, sorry not to have seen it before): you actually need to release
>> GFiles with g_object_unref(). I'd thought it would be quite obvious --
>> you create an object, you destroy it -- but seems not :)
> 
> It wasn't clear an object was being created when I checked the docs
> originally. I just looked at the object hierarchy for GFile and it
> descends from GInterface, not GObject. Is g_object_unref () callable on
> a GInterface? I guess so but it would be nice to find some concrete
> information about it ;-)
Well, hum... GObject is a "classic" OO "language" -- it's actually a
library, but still. An interface (GInterface [1]) cannot be instantiated
by itself. You can only get an "instance" of a GInterface by getting an
instance of an object (GObject [2]) that implements this interface.

So, GFile is the main interface of GIO to handle files: it is what you
use in the code. But there are many objects that implement it --
GLocalFile is the most basic implementation. These implementations are
not exposed to the user, and most of them are in GVFS AFAIK, as (a part
of?) the backends.

g_file_new_for_path() actually calls g_vfs_get_file_for_path() with the
default VFS (g_vfs_get_default()), which try to create the most
appropriate object for the given path, depending on which GVFS backends
are available.
This object is likely to be a GLocalFile, but may also be any other
object implementing GFile -- I don't know their name, but we could
imagine GDavFile, GSambaFile, GSshFile or whatever: each would implement
all the GFile interface in the way that fit to a particular goal (file
on DAV, Samba, SSH, whataver).

> Also, the docs for g_file_new_for_path don't mention releasing
> resources, but the docs for g_file_get_parent *do* say to call
> g_object_unref. Both functions return a GFile. Is this a bug in the docs
> for g_file_new_* ?
Yes, it is. Actually I personally don't think it is a real bug because I
think there is a difference between these two function:
 * g_file_new_for_path() is named "new" so it suggests it creates
something new (just like g_object_new(), g_socket_new(), etc.).
 * g_file_get_parent(), however, takes a GFile in argument, and returns
another one (the parent, right). It would be possible to think that the
original file keeps the parent around, and so may not want the caller to
release it.

And -- even if this is not really human-readable, I agree -- the return
value of g_file_new_for_path() is marked as "transfer full" which means
(thanks to the info popup) "Free data after the code is done". OK, the
free function isn't explicitly given, and it's not really consistent to
give it for g_file_get_parent() and not g_file_new_for_path().

So yes, it's kinda bug.

>> So, here's a (very) small patch for this.
> 
> I haven't applied it just yet, I'll wait for your reply to the above
> issue ;-)
I hope my explanations are clear enough for you to decide :)

Regards,
Colomban

[1]
http://library.gnome.org/devel/gobject/unstable/gtype-non-instantiable-classed.html
[2]
http://library.gnome.org/devel/gobject/unstable/gtype-instantiable-classed.html