New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make config_write more atomic to prevent truncation when out of space #871

Merged
merged 1 commit into from Apr 24, 2018

Conversation

Projects
None yet
2 participants
@dequis
Copy link
Member

dequis commented Apr 8, 2018

This is modeled after glib's g_file_set_contents. It doesn't use that
function directly because the writing is done with GIOChannel
streaming-like writes and g_file_set_contents expects the whole thing to
be in-memory.

Main differences with g_file_set_contents:

  • complete lack of win32 special casing (cygwin/WSL should work though)
  • no fallocate() (linux only, but we don't know the size upfront, anyway)
  • always calls fsync (glib skips it on btrfs or when not overwriting)

Other than that, it's the same old mkstemp + fsync + rename.


Fixes #817

In #817 (comment) I mentioned I wasn't sure if i should keep copying g_file_set_contents(). Few months later, no one is going to refactor config writing to be in-memory, so finishing that WIP code is obviously better than nothing.

The WIP code used g_rename, g_unlink, but those require including gstdio.h and their implementation is all win32 portability code so mehhhh.

Make config_write more atomic to prevent truncation when out of space
This is modeled after glib's g_file_set_contents. It doesn't use that
function directly because the writing is done with GIOChannel
streaming-like writes and g_file_set_contents expects the whole thing to
be in-memory.

Main differences with g_file_set_contents:

- complete lack of win32 special casing (cygwin/WSL should work though)
- no fallocate() (linux only, but we don't know the size upfront, anyway)
- always calls fsync (glib skips it on btrfs or when not overwriting)

Other than that, it's the same old mkstemp + fsync + rename.
@ailin-nemui

This comment has been minimized.

Copy link
Contributor

ailin-nemui commented Apr 16, 2018

@ailin-nemui ailin-nemui merged commit 0caf884 into irssi:master Apr 24, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment