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

Lex Trotman elextr at xxxxx
Tue Nov 9 23:13:29 UTC 2010


Hi again Nick,

On 10 November 2010 00:33, Nick Treleaven <nick.treleaven at btinternet.com> wrote:
> On Sat, 6 Nov 2010 11:33:41 +0200
> Dimitar Zhekov <dimitar.zhekov at gmail.com> wrote:
>
>> I'll write a non-GIO variant first, as a proof of concept.
>
> First, I feel quite reluctant to use non-GLib/GIO functions for writing
> instead of g_file_replace_contents because if there is a better
> implementation, why don't they add it?

Don't know why they havn't fixed it, but looking at the bug file it
seems they can't agree on the interface for the fix and then it all
went quiet.

>
> Also, g_file_replace_contents does have a make_backup argument we could
> provide an option for. This might handle the disk exhaustion problem.

To summarise the long thread for you Nick, g_file_replace_contents and
g_file_replace can't be used because there is NO WAY to stop
g_stream_close doing the rename of the temp file over the old file
even if the write to the temp file fails.  So you can get a broken
output file.

My post with all the steps is simply a copy of what g_file_replace,
g_stream_write and g_stream_close do, but allowing us to not rename a
broken temp file over our old file.

Whilst this is complicated, it is ****really important**** because
failing to write the output safely defeats the purpose of an editor.

We can't use GIO since there is no way of getting a writable stream
except g_file_replace, at least that I can find.

Note: Gedit uses g_file_replace and an implementation detail of
g_stream_close to hack around the problem.  Big comment in the source.
 If you want the maintenance headache of checking every release of GIO
has the same implementation then you can use this as well.

>
>> The current non-GIO is buggy anyway. First:
>>
>> if (G_UNLIKELY(len != bytes_written))
>>         err = errno;
>>
>> but fwrite() is not guaranteed to set errno, only write() is.
>>
>> Second, and more important, the result of fclose() is not checked, for
>> a buffered file stream. On lack of disk space, on my system fwrite()
>> happily returns written == len, but fclose() fails. YMMV.
>>
>> If not anything else, we should use non-buffered I/O, with fsync(), and
>> check the result of close() anyway.
>
> Thanks for finding these problems. I accept fclose should be checked.
>
> I'm not sure why write is better than fwrite - POSIX says both set
> errno.

And man on Linux says fwrite does not set errno!!!!  Unix standards
are great, everyone can have one :-S

You would have to enforce POSIX behavior and I'm not sure thats portable.

Cheers
Lex

> http://www.opengroup.org/onlinepubs/009695399/functions/fwrite.html
>
> Can you explain why non-buffered I/O is better?
>
> Nick
>
> P.S. Thanks for sending the updated patch.
> _______________________________________________
> Geany-devel mailing list
> Geany-devel at uvena.de
> http://lists.uvena.de/cgi-bin/mailman/listinfo/geany-devel
>



More information about the Devel mailing list