Re: g_file_write()

["Alexis S. L. Carvalho" <alexis cecm usp br>]

Thus spake Soeren Sandmann:
> Soeren Sandmann <sandmann daimi au dk> writes:
> 
> > but then there is a race condition with another thread setting
> > umask. This can be avoided by doing it in a separate process:

...

> > Is there a simpler way to do this?
> 
> If there is, I don't know it. I have put up a patch to do the chmod()
> in a child process:
> 
>         http://www.daimi.au.dk/~sandmann/umask-fork.patch

This looks somewhat heavy-handed, but I don't have a better idea if you
want to keep the thread safety.

Except for the lack of error handling in the call to waitpid, the patch
looks fine to me.  If you want to avoid the "waitpid got called in a
signal handler before I had a chance to do anything"-problem in the
single-threaded case, you can use sigprocmask to block SIGCHLD before
the fork and restore the previous signal mask after the waitpid - but
this won't be enough in the presence of other threads.

Also, this comment from libgsf is a bit worrying:

        /* Save to a temporary file.  We set the umask because some (buggy)
         * implementations of mkstemp() use permissions 0666 and we want 0600.
         */

It essentially means that, with these implementations, you have a small
race during which anybody could open your newly created file for
writing.  But maybe the fix for this would be to not define HAVE_MKSTEMP
in these plataforms and have GLib use its own implementation of
mkstemp...

Since you're being careful about the permissions when the file is
created, you should probably also try to keep the permissions if the
file already exists - maybe even the ownership?

> In addition I took a look at the stdio output from libgsf. One thing
> that may be worth considering adobting is the symlink following code
> they have. Currently g_file_replace() will replace the symlink instead
> of the linked file, which will probably surprise some people.

> Maybe we could even add it as g_follow_symlik() to avoid duplication
> of code in libgsf.

Sounds good to me.

Another potentially interesting thing from the libgsf code is the test
to see if the file is writable, which, even though is racy, can avoid
some bad surprises.

Alexis