[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r1680819 - /subversion/trunk/subversion/libsvn_fs_fs/revprops.c

From: Julian Foad <julianfoad_at_btopenworld.com>
Date: Tue, 26 May 2015 16:57:23 +0100

I (Julian Foad) wrote:
> Just a couple of comments, from a quick read of the patch.
>
> +/** Flushes all available internal buffers in a generic @a stream. If
> + * @sync is non-zero, it causes any buffered data to be written to the
> + * underlying device (for example, to the disk for a file stream).
>
> I'm not sure about having the 'sync' flag. What does that mean for a
> non-disk stream? Does the API really need to support two different
> ways of flushing a generic stream? I feel probably not. If we need to
> support two different ways of flushing a disk stream, could we achieve
> that in a better way? Perhaps the code that opens the disk stream
> should specify this option when opening the disk stream, rather than a
> generic writer specifying this option when it calls 'flush'?

Closing a stream always implies first flushing its own buffers. This
proposed svn_stream_flush() API supports flushing a generic stream's
buffers at any (and multiple) points in time. That's potentially
useful, but the present use case only requires us to sync a disk
file's data to disk when closing it. That's a slightly different
thing.

I note that in the MSDN documentation that Evgeny linked to, the plain
'Stream' class has a plain 'Flush' method, while the 'FileStream'
class has (in addition) a Flush(bool) method that supports a 'sync to
disk' option.

Another small point. Ivan mentioned at the beginning of this thread:

> 3. It's unclear how FLUSH_TO_DISK flag should interact with DISOWN=TRUE.

The current patch chooses one of the two possible behaviours for
svn_stream_disown() -- it chooses to forward all flush() method calls
to the 'disowned' stream, and only blocks the close() method. Nothing
wrong with that; I'm just noting that one behaviour was chosen.

- Julian
Received on 2015-05-26 18:01:07 CEST

This is an archived mail posted to the Subversion Dev mailing list.