On Fri, Nov 30, 2001 at 12:00:26AM -0600, Karl Fogel wrote:
> Kevin Pilch-Bisson <kevin@pilch-bisson.net> writes:
> > Consider the way an update works if there are no local mods. It copies the
> > text-base file to the tmp area, applies the txdelta, and then moves it back to
> > text-base and copies it out to the working copy. Note that this means it
> > starts with the supposedly read-only text-base file. If we don't change the
> > permissions, then we run into problems moving stuff around. Also even if we
> > didn't we wind up with a read-only file in the working copy. However, if we
> > just set the permissions, without worrying about the umask, then what do we
> > set them to? We could set them -rw-------, but then people have to re-set
> > the permissions to allow group reading every time the do an update. We can't
> > set them -rw-r--r--, because they might contain information that a user doesn't
> > want to be shared. So overall our best strategy is to move or copy the file,
> > then set the permissions to look like we just created the file, which means
> > obeying the umask.
> >
> > There are however, two things I don't like about this patch.
> >
> > 1) It resets a file to non-executable after an update, even if the user had
> > made it executable (e.g. autogen.sh). Not a big deal in the long run, since
> > sometime (hopefully soon), we will know that a file should have the execute
> > bit set, and set it.
>
> Bingo.
>
> What we should do, in the case of copies over existing working files,
> is mimic the permissions of the file being replaced. (Unless the
> update also sets the execute bit via properties or something, but
> that's another story and isn't a concern yet.)
>
First lets go over permission semantics with the unix mv and cp commands.
mv - Permissions are the same as source.
cp - Permissions are the same as dest if dest exists, otherwise as if
file was opened (hence obeys umask).
We could acheive the same semantics, but it would require first stat'ing the
dest file to see if it exists, and storing the permissions if it does.
Our current svn_io_copy_file (and apr_copy_file, since it lives in io.c), set
the permissions to those of the source file regardless. I'll see if I can
work up a patch to implement a read-only working copy based on this, but I'm
afraid we might run into problems when try to mv over files in text-base from
tmp/text-base, since text-base will be read-only. This would mean we would
need to stat the file, make it writable, mv the file, then set it back to
its original perms.
> When creating a new file, then I guess best bet is to pay attention to
> umask and whatever the directory inheritance conventions are (I can't
> remember them off the top of my head, but I think there are some...?)
>
open() should deal with that automatically...
>
>
> > 2) There is a race condition in svn_io_set_permissions for multithreaded
> > apps, with regard to the umask. Just wondering if anyone had any ideas of
> > how to fix that for apr. I was thinking of making:
> >
> > apr_status_t apr_umask_get(apr_fileperms_t *mask);
> >
> > and
> >
> > apr_status_t apr_umask_set(apr_fileperms_t *mask);
> >
> > Then it would be possible to perform a umask(mask=umask(0777)) in
> > apr_initialize (before any threads are created) and store the umask somewhere.
> > Then apr_umask_get would just return the value, and apr_umask_set could set
> > both apr's copy and call umask to set the new value.
> >
> > Sound okay???
>
> Hmm, don't quite understand the bit about the race condition and the
> proposed solution, may need more explanation... What exactly is the
> problem? I thought umask was something APR will be getting from the
> user's environment?
>
umask() is actually a system call, which both sets and returns the current
process umask. Thus the only way to retrieve the umask is to do the
equivalent of: umask(mask=umask(0777)).
It is possible in a multithreaded app to call open between the two umask calls
meaning the file will be created with a bogus umask.
The only safe ways I see around this in creating apr_umask_[get/set] are:
1) Put mutexes around the umask calls, AND all calls to open where O_CREAT are
specified, or
2) Have apr record the umask before any threads are running, and use that for
get operations, and have set record the new value and set the umask.
I'm open to either, but I thought that approach 2) causes a fixed amount of
memory increase (4bytes), in an apr app, whereas 1) introduces a run-time
overhead everytime a file is opened.
--
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
Kevin Pilch-Bisson http://www.pilch-bisson.net
"Historically speaking, the presences of wheels in Unix
has never precluded their reinvention." - Larry Wall
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
- application/pgp-signature attachment: stored
Received on Sat Oct 21 14:36:50 2006