On Tue, Apr 10, 2007 at 04:27:08PM -0700, Eric Gillespie wrote:
> > There's a race between the stat() and open() - if the activity was
> > reassigned to a new transaction with a longer name between the
> > two calls, we'd read a partial transaction name. Perhaps either
>
> I didn't think of that.
>
> > terminate the transaction name with something ('\n'?) and check for
> > that, or simply try to read an extra byte (and keep the length/read
> > verification), or maybe just use fstat() [which might even be more
> > efficient anyway, and is certainly easier to understand].
>
> Hm, how does fstat avoid the problem? Since i currently just
> write over the file when storing an activity, it seems to me it
> would have the same problem. If i change to renaming over the
> file, this problem goes away, but i now have to check for ESTALE.
>
Heh, you're right - in _my_ head you were already renaming the file :-)
(what, you're not in my head?)
If you rename the file, you still have a race between the stat() and
open(), so you should fstat() the file you open()ed instead.
> > > + _CHECK_STATUS;
> > > + if (bytes_read !=3D finfo.size)
> > > {
> >
> > Worth checking for zero-sized files as well?
>
> Yes (unless we switch to rename).
>
I was thinking more in terms of detecting an invalid entry - say one
where you've created the file, but then aborted before you can write the
transaction name.
> > This should write to a temporary file and rename() so that readers don't
> > see partially-written state. Of course, that also means that the code
> > should deal with ESTALE as well.
>
> I thought that since i pass the whole transaction name in one go
> that would not be a problem, but now i think about interrupted
> writes, so i guess that's not true after all. So yeah, i'll move
> the ESTALE macros to svn_error.h and rework this stuff.
>
Thanks!
Regards,
Malcolm
- application/pgp-signature attachment: stored
Received on Wed Apr 11 10:21:38 2007