! Please note that this is a snapshot of our old Bugzilla server, which is read only since May 29, 2020. Please go to gitlab.xfce.org for our new server !
Saving is not atomic
Status:
RESOLVED: MOVED
Product:
Mousepad
Component:
General

Comments

Description Dmitry Chestnykh 2012-04-22 11:21:46 CEST
Here are the problems with saving files (file_save_real function) in Mousepad:

 * It overwrites the original file. If there's a crash/power outage/other failure, the original file may get corrupted.
 
 * The result of fclose() is not checked. If there's an error flushing data on disk, it will not be reported to the user.

 * There's no fsync() before fclose(). The overwritten file is still not on disk for some period of time. If there's a power outage, the original file may get corrupted.

Suggestion on how to do it properly:

  1. Create a temporary file _in the same directory_ and open it (Check for errors).
  2. Write to this temporary file. (Check for errors.)
  3. fsync(). (Check for errors.)
  4. fclose(). (Check for errors).
  5. rename temporary file to the original file. (Check for errors).
Comment 1 Dmitry Chestnykh 2012-04-22 12:00:59 CEST
Created attachment 4352 
Check for errors in fclose(). Use fsync(). (NOT TESTED)

I've renamed the issue to "saving is not atomic" to better describe what I'm talking about in the second part. However, error checking is a different issue.

I have attached a strawman patch with error checking and fsync(), but not atomic saving. Note that I haven't even try to compile it. There's also not feature-test like HAVE_FSYNC and HAVE_FILENO.
Comment 2 Steven Jackson 2014-09-09 01:34:13 CEST
Created attachment 5643 
Proposed fix
Comment 3 Steven Jackson 2014-09-09 01:35:27 CEST
Good find, I expect this hasn't been fixed yet because no complete patch been submitted yet, so I've added one. I've used g_file_set_contents as it is cross platform and atomic.
Comment 4 Dmitry Chestnykh 2014-09-09 01:41:08 CEST
(In reply to Steven Jackson from comment #3)
> Good find, I expect this hasn't been fixed yet because no complete patch
> been submitted yet, so I've added one. I've used g_file_set_contents as it
> is cross platform and atomic.

Looks good to me: simple and safe (I didn't know about g_file_set_contents, which saves atomically), thanks!
Comment 5 Theo Linkspfeifer editbugs 2020-05-22 21:39:19 CEST
*** Bug 16810 has been marked as a duplicate of this bug. ***
Comment 6 Git Bot editbugs 2020-05-24 01:27:19 CEST
-- GitLab Migration Automatic Message --

This bug has been migrated to xfce.org'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.xfce.org/apps/mousepad/-/issues/4.

Please create an account or use an existing account on one of our supported OAuth providers. 

If you want to fork to submit patches and merge requests please continue reading here: https://docs.xfce.org/contribute/dev/git/start#gitlab_forks_and_merge_requests

Also feel free to reach out to us on the mailing list https://mail.xfce.org/mailman/listinfo/xfce4-dev

Bug #8754

Reported by:
Dmitry Chestnykh
Reported on: 2012-04-22
Last modified on: 2020-05-24
Duplicates (1):
  • 16810 Partial data loss when partition is full

People

Assignee:
Matthew Brush
CC List:
5 users

Version

Target Milestone:
Mousepad 0.2.x

Attachments

Check for errors in fclose(). Use fsync(). (NOT TESTED) (1.58 KB, patch)
2012-04-22 12:00 CEST , Dmitry Chestnykh
no flags
Proposed fix (8.72 KB, patch)
2014-09-09 01:34 CEST , Steven Jackson
no flags

Additional information